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

Splits with merge option for dock panels #582

Merged
merged 6 commits into from
Jun 13, 2023

Conversation

tavin
Copy link
Contributor

@tavin tavin commented May 7, 2023

The point of this new feature is to avoid the following situation:
Screenshot

I think it's a widely accepted convention to work in a layout of 3 panels: file tree, text editor, and preview. But if launching the preview always splits the editor widget, you keep winding up with 2 preview splits and you repeatedly have to drag one of the preview tabs over to the adjacent split.

To solve this I have introduced 4 new insert modes (merge-left, merge-right, merge-top, merge-bottom) which function like (split-left, split-right, split-top, split-bottom) but any existing splits will be reused. The behavior effectively changes to tab-after on the tab layout found to the left/right/top/bottom.

See the dockpanel example which now has a context menu for split and merge operations on the content widgets.

@welcome
Copy link

welcome bot commented May 7, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@krassowski krassowski added the enhancement New feature or request label May 11, 2023
Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Thank you, I agree that this is a nice improvement.

  1. To fix the failing test, you can run yarn run api from top level directory and commit the changes.
  2. Would you be able to add some minimal tests for the newly added modes in docklayout.spec.ts?

describe('#addWidget()', () => {
it.skip('should have some tests');
});

@tavin
Copy link
Contributor Author

tavin commented May 11, 2023

sure @krassowski maybe give me a few days, but I'm happy to address the fit & finish

@tavin
Copy link
Contributor Author

tavin commented May 22, 2023

@krassowski all the checks have passed now except macos/firefox which I guess is a known issue

Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Thank you @tavin!

@tavin
Copy link
Contributor Author

tavin commented May 25, 2023

@krassowski You're welcome. I presume someone will merge this (I don't have merge access)?

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @tavin

@fcollonval fcollonval merged commit 3f0be5c into jupyterlab:main Jun 13, 2023
@welcome
Copy link

welcome bot commented Jun 13, 2023

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

gabalafou pushed a commit to gabalafou/lumino that referenced this pull request Jul 7, 2023
* dock insert modes allowing reuse of existing splits

* splits and merges in dockpanel example

* revise description of merge modes

* yarn ran api

* rudimentary tests for addWidget()

* ¯\_(ツ)_/¯
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants