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.

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

add overriding tab bar and allowed splits on a per node and surface basis #253

Draft
wants to merge 1 commit into
base: release-0.15
Choose a base branch
from

Conversation

0xcaff
Copy link

@0xcaff 0xcaff commented Oct 13, 2024

these changes are made in a backwards compatible way.

i'm looking for feedback on naming, i think it could be more clear. maybe docking override?

also maybe the example could be made a bit more clear? maybe i should move it out of hello or put it behind a flag?

…asis

these changes are made in a backwards compatible way.

i'm looking for feedback on naming, i think it could be more clear. maybe docking override?

also maybe the example could be made a bit more clear
@Adanos020 Adanos020 changed the base branch from main to release-0.15 October 14, 2024 16:08
Copy link
Owner

@Adanos020 Adanos020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this design overcomplicates how this library is used because it now has two places where allowed splits are controlled. What I'd do instead is get rid of the AllowedSplits from DockArea and instead make them overrideable from TabViewer only. This way, if someone wants to change the allowed splits for all tabs, they can just return one value from TabViewer::allowed_splits unconditionally.

This would mean:

  • getting rid of the AllowedSplitsOverride enum altogether;
  • removing DockArea::allowed_splits field and builder functions;
  • changing the return value of TabViewer::allowed_splits to Option<AllowedSplits>, where None would represent the current NoDock variant.

Does this sound good?

@0xcaff
Copy link
Author

0xcaff commented Oct 14, 2024

I agree. I avoided this design as it involved making a breaking change. If we're happy to break the API, I'm happy to do things this simpler way.

Also, as posted in discord:

After thinking about it a bit, the concept needs a bit more work. I decided to vendor the dependency for now and make whatever changes to understand what exactly I'm trying to do. Currently, if all the tabs are closed from a pane and the parent node is no dock, it will not be possible to open that pane again. This is kinda a weird UX.

Do we want to proceed with the proposed changes or spend more time figuring out what is ideal?

@Adanos020
Copy link
Owner

I think we should prioritise usability over backwards compatibility. Especially since we're still in 0.x 😉
NoDock could also be a separate feature, worked on in a different PR if it's a bit of a pain to design properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants