Improve the size policy (and more and better icons) by mgrojo · Pull Request #1684 · sqlitebrowser/sqlitebrowser
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
These changes improve the size policy for translations having long texts
in some buttons by:
- Converting the text buttons in the Edit Database Cell to icons
- Making the "Type of data" label wrappable and expandable
- Converting the text buttons in the Browse Data tab to icons
- Allowing the Plot combo-boxes to shrunk
All this allows both the Browse Data and the docks to grow and shrink with
more freedom.
New icons for buttons are reused when appropriate in context menus.
Added icon for filter and improved icon for docks (has been mirrored so it
matches the actual dock position).
Added Print icon in Edit Database Cell using the extra free space, so the
print action there is more visible.
This continues the effort started in #1678.
These changes improve the size policy for translations having long texts in some buttons by: * Converting the text buttons in the Edit Database Cell to icons * Making the "Type of data" label wrappable and expandable * Converting the text buttons in the Browse Data tab to icons * Allowing the Plot combo-boxes to shrunk All this allows both the Browse Data and the docks to grow and shrink with more freedom. New icons for buttons are reused when appropriate in context menus. Added icon for filter and improve icon for docks (has been mirrored so it matches the actual dock position). Added Print icon in Edit Database Cell using the extra free space, so the print action there is more visible. This continues the effort started in #1324.
@zvova7890 You might also like to review the result.
Visually, this looks good to me. 😄
Haven't taken a look at the code nor tried it out in operation though. 😉
I've been thinking on converting all these packed buttons that we have for real toolbars. They are more flexible and always can be shrinked because they start adding the buttons to a popup menu in a final double arrow. I was unable to add the toolbars with qtcreator but it was possible manually editing the ui file. After that they can still be modified using qtcreator and the buttons can be added to the toolbars (but they have to change from QToolButton to QAction). In this way we could allow minimal width without reducing the size of combo-boxes.
This provides more flexibility, like the way how toolbars are compacted when they have not enough space. The QToolButtons in Browse Data tab and in Edit Cell dialog are converted to QActions and inserted in new toolbars embedded in the same place as the old buttons. Everything else should stay the same (shortcuts, tool-tips and what's-this information).
The combo-box used for the main toolbar is replicated for all the toolbars in application. In this way, users with high resolutions can use the styles with both icons and text, while users with lower resolutions can leave the default styles, which should be better for them. Some icon texts has been abbreviated from their default values, so they fit better in the toolbars when they are visible. The print icon in Edit Cell has been moved to the right, where it would be the first to be collapsed. The original what's-this info for Set as NULL in Edit Cell toolbar has been restored.
I've converted the buttons in Browse Data tab and the Edit Database Cell into toolbars.
This is how it looks if there is not enough horizontal space:
I think it's useful to allow customizing the style of all our toolbars, so we give a default that it's useful for low and medium resolutions, but the user with higher resolutions can set the icon with text styles in order to have bigger buttons:
This is how it looks with the text beside icons in all the toolbars:
Some feedback about these changes? It doesn't need a complete review or test, only opinions about whether these new toolbars is the way to go.
Creating once-off builds for this at the moment, so people can try it out and give hands on feedback. 😄
Noticed one minor warning in the macOS build output which sounds like it might be important - probably not from this PR's size_icons branch, probably from code in master:
In file included from MainWindow.cpp:2:
In file included from .ui/ui_MainWindow.h:40:
./ExtendedTableWidget.h:59:10: warning: 'selectAll' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
void selectAll();
^
/Users/jc/Qt/5.11.3/clang_64/lib/QtWidgets.framework/Headers/qabstractitemview.h:236:18: note: overridden virtual function is here
virtual void selectAll();
^
Not sure if that means the selectAll() call will be calling the right function from here or not. Figured it's better to mention than not though. 😉
Uh oh. From watching the build logs, it turns out the last few nightly builds for both osx and win have been getting mucked with the merge of an older commit (61596ad) I manually rewound and re-applied slightly differently.
Not sure how well the builds since then have really been representing the changes (!).
Well, it's fixed now, so the nightlies should again be using a not-muddled-up version of master.
I'll redo the nightlies for today after these once-off builds too, just for good measure. 😄
Any feedback about this change? If nobody is opposed, I think I'll merge this in the following days. It shouldn't be a problem, if there is some issue, we can fix it later or even revert some of the changes if they aren't welcome (I don't think they will, but someone might prefer the previous approach in some aspect).
I haven't looked though all the changes (especially in the ui files) but this looks pretty good to me. So no objections from my side to merge this 😄 The only thing I noticed is that I got a warning that the static void addShortcutsTooltip(QWidget* widget, const QList<QKeySequence>& keys) function overload isn't used anywhere and can be deleted.
Oh nevermind, that warning is unrelated to this PR 😄 Well not entirely, before this PR the function is used, with this PR it isn't. So it does make sense to remove it in this PR 😉
The addShortcutsTooltip function applying to QWidget was no longer used after having converted all the buttons to actions, so it is removed.
Thanks for reviewing, @MKleusberg. I've deleted the obsolete function and merged this.







