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

Maximize Pane where mouse is shortcut by turtletooth (modified) #581

Merged
6 commits merged into from Jul 12, 2016

Conversation

shun-iwasawa
Copy link
Member

This PR is based on the PR #404 by @turtleTooth . It remain basically unchanged from the original PR, except small modifications as follows:

  • Regarding the function MainWindow::maximizePanel(); changed to get cursor position (QPoint p) even when the panel is maximized in case of the mouse cursor being outside of the main window. Accordingly I removed the flag panelMaximized which seems to be unused anymore.
  • Regarding the function MainWindow::fullScreenWindow(); changed to call functions directly from MainWindow itself.
  • Changed the command name tentatively from "Toggle Full Screen" to "Toggle Main Window's Full Screen Mode" in order to distinguish from "Show/Hide Full Screen" command which is already available in the viewer and the flipbook. Please let me know if anyone have a good idea for the command name.
  • Applied clang format.

The original comment for the PR #404 by @turtleTooth is as follows:

I added the ability to set a mouse shortcut to maximize the pane where the mouse is. For example, if the mouse is in the xsheet, clicking ` (back tick) will make the xsheet maximized. The same works for the scene viewer and the palettes. . .
This should take care of #335

Edit-
I added the ability to maximize an entire room with Ctrl + `
This is #336

These shortcuts can be changed of course

Demos:
https://youtu.be/80EF-2QG-gE

https://youtu.be/3ykJ-88BftI

@turtleTooth , thank you very much for the improvement!

@shun-iwasawa
Copy link
Member Author

Jenkins

@shun-iwasawa
Copy link
Member Author

@turtleTooth
Could you please review my modification whether it follows your original intent? Thanks.

@ghost
Copy link

ghost commented Jul 5, 2016

This looks good. "Toggle Main Window's Full Screen Mode" seems a little long. What do you think about Toggle Program Full Screen or Toggle Full Screen Program.
Another thing that could be considered (that I didn't think of until now) - should there be a way for users to discover this functionality other than browsing the keyboard shortcuts? I realize that the menu structure is already pretty full, but it's worth considering.

@blurymind
Copy link

blurymind commented Jul 5, 2016

~ seems like a good default keyboard shortcut. Its usually the TAB key in other software.

As a user - that is how I discover it. Knowing that if available - its likely to be the tab key.

@ghost
Copy link

ghost commented Jul 6, 2016

Right now, the default shortcut is the ~ key (same as the `). I took this from Premiere which uses the same shortcut. It looks like tab actually is used to tab around the interface.

@blurymind
Copy link

` is a good default too! It is close to tab and understand if tab is reserved already.

I dont think in this case we need a menu entry for this command, but if there is one - where would you put it? The mouse needs to be over the window that is going to be maximized

@ghost
Copy link

ghost commented Jul 6, 2016

Good point. I'm not sure where a command would go. Maybe someday once shortcuts are finalized, a shortcut reference can be made available.

@blurymind Do you have any thoughts on the name of the command? I think keeping them brief if possible is best, but still clear.

@skitaoka skitaoka added the UI label Jul 6, 2016
@blurymind
Copy link

blurymind commented Jul 6, 2016

@turtleTooth How about:

"Maximize viewport" (a viewport is not a window - a window contains viewports)
"Toggle maximize viewport" (this makes more sense because the command is really a toggle)

"Toggle fullscreen area" (thats how Blender calls it - they use the term "Area" instead of viewport)


createMenuWindowsAction(MI_MaximizePanel, tr("Toggle Maximize Panel"), "`");
createMenuWindowsAction(MI_FullScreenWindow,
tr("Toggle Main Window's Full Screen Mode"),
Copy link

Choose a reason for hiding this comment

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

What about changing the existing full screen to "Toggle Full Screen Canvas" so this can be "Toggle Full Screen"?

@ghost
Copy link

ghost commented Jul 12, 2016

Since the only thing that might change is the name of the command, but the implementation is good, I think it is good to merge. If the name should be changed, it can happen later.
Thanks @shun-iwasawa for the PR.

@blurymind
Copy link

good thinking. It will be easier to decide that way I think.

@shun-iwasawa shun-iwasawa deleted the turtletooth-maximize branch December 1, 2016 01:36
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants