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

Feature: zoom in/out buttons and menu items #1266

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Serge45
Copy link
Contributor

@Serge45 Serge45 commented Aug 9, 2020

As requested in #1256, I added zoom in, zoom out and reset zoom items to the edit menu. Two convenient tool buttons for zoom in and zoom out were added to browser tab as well.

@Serge45 Serge45 requested a review from trollixx as a code owner August 9, 2020 15:04
@trollixx
Copy link
Member

Thanks for looking into this. I think adding more buttons to the toolbar results in UI clutter. Nowadays major browsers moved to one menu button. I'd say Zeal should follow the trend. That would also allow us to address #1251.

Would you be willing to give menu button try?

@Serge45
Copy link
Contributor Author

Serge45 commented Aug 15, 2020

Thanks, this is a good idea, I will move those buttons to a menu button ASAP.

Copy link
Member

@trollixx trollixx left a comment

Choose a reason for hiding this comment

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

Looks good! I've added a few minor requests. Also please squash and rebase your changes. Thanks!

src/libs/ui/browsertab.cpp Outdated Show resolved Hide resolved
src/libs/ui/browsertab.cpp Outdated Show resolved Hide resolved
src/libs/ui/browsertab.cpp Show resolved Hide resolved
src/libs/ui/mainwindow.ui Outdated Show resolved Hide resolved
src/libs/ui/mainwindow.ui Outdated Show resolved Hide resolved
src/libs/ui/mainwindow.ui Outdated Show resolved Hide resolved
src/libs/ui/mainwindow.ui Outdated Show resolved Hide resolved
@Serge45 Serge45 force-pushed the feat/zoom-buttons branch 2 times, most recently from b1b3c74 to 1957dd2 Compare September 6, 2020 03:21
Copy link
Member

@trollixx trollixx left a comment

Choose a reason for hiding this comment

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

Nice! Thank you! I have some nitpicks and small things I'd like to try. Is it okay if push my changes directly to this PR? I can also leave a detailed review but that might take longer.

@Serge45
Copy link
Contributor Author

Serge45 commented Sep 8, 2020

Ok, sure. There are some ugly codes to make widget in QWidgetAction behave like widget for QAction in QMenu, but I haven't found a better way to implement it :(.

An event filter was also added to prevent pop-up menu closing while
zoom in/out actions triggered.
A custom QStyleSheet must be applied to BrowserZoomWidget
since QMenu does not handle item highlighting of QWidget
in QWidgetAction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants