fix: splitter not observe 'size' and 'closed' values changes

[hamed-musallam]

• Splitter needs to observer the changes in the initialSize and initialClosed properties. #452

[targos]

/cc @stropitek given you recently had a splitter library on your radar.

[stropitek]

/cc @stropitek given you recently had a splitter library on your radar.

The library I was looking at is:
https://github.com/bvaughn/react-resizable-panels

I looked at it and one of the main limitation is that all sizes are expressed in percent. It does not seem possible to set a default panel size in pixel units, which I think we do in NMRium.

[targos]

bvaughn/react-resizable-panels#47 (comment)

Please also review!

[stropitek]

Please also review!

I don't think I'm ok with the API changes. I don't understand the point of having both size / initialSize, closed / initialClosed for properties which are uncontrolled.

The issue also is not very helpful because it describes the solution and not the problem.

@lpatiny will show me the root cause of the bug and I will make a second assessment.

@hamed-musallam if you could explain the original problem in the issue that would be helpful. A repro as well!

[hamed-musallam]

Currently, in NMRium we can control the size of the split panel from the general settings and it is per workspace and we always update the value once the resize ends, the problem came when we switch between workspaces or change from the general setting the Splitter will not recognize the new value and only way is by forcing the Splitter to re-render by change the key value, The same applied to the closed property.

if we always change the key value this lead to re-render all the children.

What we have right now in this PR, is we keep default properties initialClosed and intialSize, and if we want to control these values from the parent component we can use size and closed properties

Take a look at this comment #452 (comment)

[stropitek]

Take a look at this comment #452 (comment)

Why did you choose to add properties rather than keep (or rename) the initialSize and initialClosed properties?

[hamed-musallam]

Just to keep the old component interface as it is because most of the users will not control the closed and size properties which I think is a special case in NMRium so they will hardcode the values for these properties and will not change it.

[codecov-commenter]

Codecov Report

Patch coverage: 14.81% and project coverage change: -0.09 ⚠️

Comparison is base (993f5a1) 49.49% compared to head (3a61004) 49.40%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #457      +/-   ##
==========================================
- Coverage   49.49%   49.40%   -0.09%     
==========================================
  Files         137      137              
  Lines        7356     7371      +15     
  Branches      184      184              
==========================================
+ Hits         3641     3642       +1     
- Misses       3715     3729      +14     

Impacted Files	Coverage Δ
src/components/split-pane/SplitPane.tsx	32.44% <14.81%> (-1.37%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[hamed-musallam]

I removed the initialSize and initialClosed properties and keep the size and closed properties and made the required changes, can you check the changes and if everything from your side looks good to you, can you merge it and create a new release from the package

[stropitek]

LGTM but I can't test the storybook. Next time please make sure to create a branch on the original repo rather than on a fork so that the cloudflare build works.