Potential changes for major revision

[voliva]

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.

[kusti8]

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 ***@***.***> 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 <#211>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJW73P88kc5AdK9Z70iwsW4inUtaQef1ks5vhYdxgaJpZM4cxoy_> .

[kusti8]

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]

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]

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