Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve SplitPaneState #3974

Merged
merged 3 commits into from
Jun 17, 2024
Merged

Improve SplitPaneState #3974

merged 3 commits into from
Jun 17, 2024

Conversation

vdshb
Copy link
Contributor

@vdshb vdshb commented Nov 25, 2023

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

@vdshb vdshb mentioned this pull request Nov 25, 2023
Closed
@vdshb
Copy link
Contributor Author

vdshb commented Dec 19, 2023

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

@MatkovIvan MatkovIvan requested a review from m-sasha January 30, 2024 09:44
@vdshb
Copy link
Contributor Author

vdshb commented Apr 22, 2024

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

@vdshb
Copy link
Contributor Author

vdshb commented Apr 29, 2024

@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
Copy link
Member

m-sasha commented Apr 30, 2024 •
edited
Loading

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
Copy link
Contributor Author

vdshb commented May 6, 2024 •
edited
Loading

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
Copy link
Contributor Author

vdshb commented May 17, 2024

@m-sasha, any feedback?

@vdshb
Copy link
Contributor Author

vdshb commented Jun 4, 2024 •
edited
Loading

@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.

@MatkovIvan MatkovIvan requested a review from igordmn June 4, 2024 09:44
@m-sasha
Copy link
Member

m-sasha commented Jun 4, 2024

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

@igordmn igordmn merged commit 67da8d1 into JetBrains:master Jun 17, 2024
@vdshb
Copy link
Contributor Author

vdshb commented Jun 17, 2024

Thanks for eventually merged improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igordmn igordmn igordmn approved these changes

@m-sasha m-sasha m-sasha approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vdshb @m-sasha @igordmn