[GH-ISSUE #211] Potential changes for major revision #147

New issue

Closed

opened 2026-05-05 11:48:16 -06:00 by gitea-mirror · 4 comments

gitea-mirror commented

2026-05-05 11:48:16 -06:00

Owner

Originally created by @voliva on GitHub (Apr 16, 2019).
Original GitHub issue: https://github.com/kusti8/proton-native/issues/211

Hey, thank you so much for this awesome library - I've been looking for something like this for so long.

However, I've seen parts of the structure of the code that I think that they can be improved, the main point being that it was a bit hard to follow what is a component doing as it uses class with method overriding, etc.

For that, I forked this repo, added some of the potential changes I thought they would make sense, and implemented just a few components to check that everything works as expected:

App
Window
VerticalBox
Entry
Area
- Area.Group
- Area.Rectangle
- Area.Bezier

I documented the reasoning and results in here: PotentialChanges - I could try to paste it down here, but it's some markdown format I think it makes more sense to use github's reader.

Please, don't take this as an attack of some sort, I think it's all good work and I just wanted to point out things where I thought there was room for improvement. I'd be more than pleased to help if you want to transition some of those ideas into your project.

Originally created by @voliva on GitHub (Apr 16, 2019). Original GitHub issue: https://github.com/kusti8/proton-native/issues/211 Hey, thank you so much for this awesome library - I've been looking for something like this for so long. However, I've seen parts of the structure of the code that I think that they can be improved, the main point being that it was a bit hard to follow what is a component doing as it uses class with method overriding, etc. For that, I [forked](https://github.com/voliva/proton-native/tree/rendererRework) this repo, added some of the potential changes I thought they would make sense, and implemented just a few components to check that everything works as expected: * App * Window * VerticalBox * Entry * Area * Area.Group * Area.Rectangle * Area.Bezier I documented the reasoning and results in here: [PotentialChanges](https://github.com/voliva/proton-native/blob/rendererRework/PotentialChanges.md) - I could try to paste it down here, but it's some markdown format I think it makes more sense to use github's reader. Please, don't take this as an attack of some sort, I think it's all good work and I just wanted to point out things where I thought there was room for improvement. I'd be more than pleased to help if you want to transition some of those ideas into your project.

gitea-mirror closed this issue

2026-05-05 11:48:19 -06:00

gitea-mirror commented

2026-05-05 11:48:23 -06:00

Author

Owner

@kusti8 commented on GitHub (Apr 16, 2019):

Hi,

This looks great and I agree that the old hierarchy is due for a change. It
worked better when there were fewer exceptions, but it increasingly turned
large. I'll look this over this weekend more thoroughly.

On Tue, Apr 16, 2019 at 4:18 AM Víctor Oliva notifications@github.com
wrote:

Hey, thank you so much awesome library - I've been looking for something
like this for so long.

However, I've seen parts of the structure of the code that I think that
they can be improved, the main point being that it was a bit hard to follow
what is a component doing as it uses class with method overriding, etc.

For that, I forked
https://github.com/voliva/proton-native/tree/rendererRework this repo,
added some of the potential changes I thought they would make sense, and
implemented just a few components to check that everything works as
expected:

App

Window

VerticalBox

Entry

Area

Area.Group

Area.Rectangle

Area.Bezier

I documented the reasoning in here: PotentialChanges
https://github.com/voliva/proton-native/blob/rendererRework/PotentialChanges.md

Please, don't take this as an attack of some sort, I think it's all good
work and I just wanted to point out things where I thought there was room
for improvement. I'd be more than pleased to help if you want to transition
some of those ideas into your project.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/kusti8/proton-native/issues/211, or mute the thread
https://github.com/notifications/unsubscribe-auth/AJW73P88kc5AdK9Z70iwsW4inUtaQef1ks5vhYdxgaJpZM4cxoy_
.

@kusti8 commented on GitHub (Apr 16, 2019): Hi, This looks great and I agree that the old hierarchy is due for a change. It worked better when there were fewer exceptions, but it increasingly turned large. I'll look this over this weekend more thoroughly. On Tue, Apr 16, 2019 at 4:18 AM Víctor Oliva <notifications@github.com> wrote: > Hey, thank you so much awesome library - I've been looking for something > like this for so long. > > However, I've seen parts of the structure of the code that I think that > they can be improved, the main point being that it was a bit hard to follow > what is a component doing as it uses class with method overriding, etc. > > For that, I forked > <https://github.com/voliva/proton-native/tree/rendererRework> this repo, > added some of the potential changes I thought they would make sense, and > implemented just a few components to check that everything works as > expected: > > - App > - Window > - VerticalBox > - Entry > - Area > - Area.Group > - Area.Rectangle > - Area.Bezier > > I documented the reasoning in here: PotentialChanges > <https://github.com/voliva/proton-native/blob/rendererRework/PotentialChanges.md> > > Please, don't take this as an attack of some sort, I think it's all good > work and I just wanted to point out things where I thought there was room > for improvement. I'd be more than pleased to help if you want to transition > some of those ideas into your project. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <https://github.com/kusti8/proton-native/issues/211>, or mute the thread > <https://github.com/notifications/unsubscribe-auth/AJW73P88kc5AdK9Z70iwsW4inUtaQef1ks5vhYdxgaJpZM4cxoy_> > . >

gitea-mirror commented

2026-05-05 11:48:25 -06:00

Author

Owner

@kusti8 commented on GitHub (May 5, 2019):

I'm currently working on a v2 with a major rewrite of the project. I want to revisit this when I get closer. I agree with your idea, I just want to clean up the code so it is easier to read.

@kusti8 commented on GitHub (May 5, 2019): I'm currently working on a v2 with a major rewrite of the project. I want to revisit this when I get closer. I agree with your idea, I just want to clean up the code so it is easier to read.

gitea-mirror commented

2026-05-05 11:48:27 -06:00

Author

Owner

@kusti8 commented on GitHub (May 5, 2019):

I'm going to be posing questions as I get to parts. About the top-level App component:

I agree that having windows there but not being showed goes against react, so I want to eliminate that. However, I don't think that eliminating it as a React component all together is a good idea. The way I see it if you have multiple windows with separate Windows being applied to a single root App by registering them similar to ReactDOM, then you loose a lot of the communication between those windows AFAIK.

I want to be as similar to the react-native syntax as possible in this rewrite. Some things will inevitably have to change since apps don't support windows etc. React-native has a single place where you register your app component: AppRegistry.registerComponent(appName, () => App);. I'm thinking of keeping that single registry, having a top level App component and then have nested Windows that can be updated like normal components (meaning no windows that are in the tree, but not shown).

@kusti8 commented on GitHub (May 5, 2019): I'm going to be posing questions as I get to parts. About the top-level App component: I agree that having windows there but not being showed goes against react, so I want to eliminate that. However, I don't think that eliminating it as a React component all together is a good idea. The way I see it if you have multiple windows with separate Windows being applied to a single root App by registering them similar to ReactDOM, then you loose a lot of the communication between those windows AFAIK. I want to be as similar to the react-native syntax as possible in this rewrite. Some things will inevitably have to change since apps don't support windows etc. React-native has a single place where you register your app component: `AppRegistry.registerComponent(appName, () => App);`. I'm thinking of keeping that single registry, having a top level App component and then have nested Windows that can be updated like normal components (meaning no windows that are in the tree, but not shown).

gitea-mirror commented

2026-05-05 11:48:29 -06:00

Author

Owner

@kusti8 commented on GitHub (May 9, 2019):

I've finished up most of the rewrite using a lot of your code and its much nicer. I can probably clean it up more later, but I'll track anything more in #213

@kusti8 commented on GitHub (May 9, 2019): I've finished up most of the rewrite using a lot of your code and its much nicer. I can probably clean it up more later, but I'll track anything more in #213