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

Collapse sidebar with Shift+Tab #4735

Closed
wants to merge 5 commits into from

Conversation

wavexx
Copy link
Contributor

@wavexx wavexx commented Sep 11, 2020

Handle Shift+Tab in both normal/preview mode to collapse the sidebar. Since Tab switches mode, Shift+Tab felt a natural extension, and is easy to use.

The special handling for shift (select rectangle, fine movement) still works as expected.

Tab and Shift-Tab are documented in the keyboard shortcut dialogs, as well as the tooltips and menus.

The tooltip of the collapse button has been changed from "Right panel" to "sidebar" for consistency, which is used everywhere else.

Fixes #4712

@lukasmatena
Copy link
Collaborator

lukasmatena commented Oct 10, 2020

@wavexx Thanks. I looked at the PR and I have a question. I did not exactly understand the flip trick and I got rid of it. Could you have a look at my branch lm_wavexx-collapse_with_shift_tab and give me your opinion on the last commit (de36314)? Did I break something?

As for the other change in that commit, I'm not sure if the shortcut should be active when the toolbar is hidden. When someone hides the sidebar by mistake through the shortcut, he might not be able to reactivate it (on the other hand, the toolbar is shown by default, so the user would have to hide it first and then forget that it is there). We will discuss this internally and I will amend the commit depending on the result.

Please, do not update the PR, it is ready to be merged. Let's discuss any possible changes first.

@wavexx
Copy link
Contributor Author

wavexx commented Oct 10, 2020

Shift-Tab should always be available in order to be useful. I'm using the shortcut to get more real-estate when looking at a model, and use it again to restore the sidebar.

The menu is always available, so that can be used if one uses the shortcut by mistake. I consider Shift-Tab to be hard-enough to press not to be an issue here.

The flip-trick was just used to avoid having two separate helper functions while setting the tooltip. Depending on which callback function is used, in one case the sidebar is already "shown", in the other is still not. I did this mostly to have a single string to translate instead of two, which I know is a pain for translators.

(I didn't look at the branch yet, I will do tomorrow)

@lukasmatena
Copy link
Collaborator

I didn't look at the branch yet, I will do tomorrow

Great, please let me know after you do. Thanks.

@lukasmatena
Copy link
Collaborator

Actually, there still was a problem. When the shortcut was used, the tooltip on the toolbar would not be updated, because the update was bound to the toolbar click only (the menu suffered the same). I have amended my commit in my branch with an attempt to fix it. I have also rebased the branch on top of current master to make sure there is no conflict with any recent changes.

You are right about the menu item being always visible, I did not realize that. In that case I agree that the shortcut should also be always available.

Let me know what you think about last state of lm_wavexx-collapse_with_shift_tab.

@wavexx
Copy link
Contributor Author

wavexx commented Oct 11, 2020

Looks and works perfectly. I tried all combinations I could think of, and the tooltip label always look correct as well 👌

@lukasmatena
Copy link
Collaborator

Thanks for the pull request and the extra testing.
Merged in 482a58c.
Closing.

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.

Keyboard shortcut to collapse the right panel
2 participants