JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
15.17k stars 1.11k forks source link

Improve SplitPaneState #3974

Closed vdshb closed 1 day ago

vdshb commented 6 months ago

Improve SplitPane programmatic configuration through changes in SplitPaneState. Add programmatic SplitPane programmatic configuration changes to demo.

vdshb commented 6 months ago

BTW, I'm not slightly insist on changing the demo-app. I just thought this change might be benificial. It might be easily reverted.

vdshb commented 1 month ago

@m-sasha , could you please review this PR?

vdshb commented 1 month ago

@m-sasha , @MatkovIvan What can I do to clarify the state of this PR? It's pretty small (tiny if you are not interested in demo-app changes), but it's ignored for quiet a while.

m-sasha commented 1 month ago

Making moveEnabled settable makes sense, but I'm not sure what the change to positionPercentage is trying to achieve. Also, I don't like the code duplication in the positionPercentage setter and dispatchRawMovement.

vdshb commented 1 month ago

I'm not sure what the change to positionPercentage is trying to achieve.

Effectively it's making positionPercentage settable as well. I've just overcomplicated it initially.

You can revert SplitPaneState and see, that new functions in changed demo-app (Set fraction V and Set fraction H) are not working without it.

I personally use it to save state of multiple SplitPanes on application close, to restore them on fresh application start.

Also, I don't like the code duplication in the positionPercentage setter and dispatchRawMovement.

Fair. It was my initial laziness to investigate how positionPercentage and dispatchRawMovement really work. I've changed it. It is much simpler now.

vdshb commented 1 month ago

@m-sasha, any feedback?

vdshb commented 2 weeks ago

@m-sasha, @MatkovIvan What can I do to make this PR closer to merge? It's become pretty simple and even smaller since the last change.

m-sasha commented 2 weeks ago

Sorry, I've just been busy with more urgent issues recently. I'll try to give it another look soon.

vdshb commented 1 day ago

Thanks for eventually merged improvement!