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

@mgrojo

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.

@mgrojo

@zvova7890 You might also like to review the result.

@mgrojo

Some screenshots:

Minimum dock size (with all the docks):
imagen

Maximum dock size:
imagen

New icons in context menu:
imagen

@justinclift

Visually, this looks good to me. 😄

Haven't taken a look at the code nor tried it out in operation though. 😉

@zvova7890

Now it very nice! Also Mode combo restrict minimum size like this(again translation is a little bigger than EN original)
image

@mgrojo

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.

@justinclift

Sounds like it'd be useful to try out. 😄

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.

@mgrojo

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:

imagen

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:

imagen

This is how it looks with the text beside icons in all the toolbars:

imagen

imagen

@zvova7890

Look's good! I like it. I will test this tomorrow

@mgrojo

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.

@justinclift

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. 😉

@justinclift

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. 😄

@justinclift

This morning's nightly builds have been regenerated for macOS and windows now too.

@mgrojo

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).

@MKleusberg

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.

@MKleusberg

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.

@mgrojo

Thanks for reviewing, @MKleusberg. I've deleted the obsolete function and merged this.

Labels