search

bimspot / xeokit-react

Integratation of the xeokit viewer into a React application.
32 stars 22 forks source link

Section planes refactor #12

Closed tothatt81 closed 4 years ago

tothatt81 commented 4 years ago

Hey Barna,

As discussed on Discord, I have some suggestions on how to refactor the section planes toggling and visibility.

In short, we could reduce the number of states that you currently have set up to track these. Also, it might be worth extracting the whole logic into its own file or perhaps even its own custom hook.

If this is all right with you, I'll prepare a PR in the coming days.

barnabasmolnar commented 4 years ago

Hello there,

Yes, that'd be great, thank you! Let's discuss that last part you mentioned a little bit more sometime because it's actually a very interesting proposition.

The useViewer file is already quite fat and is only going to grow in size as new features are added. There's most likely quite a few blocks of code that could be extracted into separate files so as to improve organisation and tidiness. And as you pointed out, it may very well make sense to organise distinct features as custom hooks.

But let's stick with the section planes refactor for now and get back to this later.