Visual sort indicators for multi-column sorting #1810

mgrojo · 2019-03-18T22:42:34Z

This adds visual sort indicators to the already working multi-column
sorting. Qt sort indicator is disabled, so only one indicator per column
is visible.

Unicode characters are used to indicate direction (triangles) and sort
column order (superscript numbers).

See issue #1761

This adds visual sort indicators to the already working multi-column sorting. Qt sort indicator is disabled, so only one indicator per column is visible. Unicode characters are used to indicate direction (triangles) and sort column order (superscript numbers). See issue #1761

justinclift · 2019-03-26T06:43:34Z

Just tried this out, and it's already a definite win. 😄

One thing that I'm not sure about, is if clicking an already selected column header should reverse that column?

For example, using the pitchfork database, and filtering the results to just "Massive Attack", I've selected three sort columns:

It "felt natural" to then click on the 2nd column again, expecting the sort on that column to be reversed. But no change to it's ordering occurred.

Does that make sense? 😄

justinclift · 2019-03-26T07:31:27Z

Building with this on Win64 (using MSVC 2017) is failing:

6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(224): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(224): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(224): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(225): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(225): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(225): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(226): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(226): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(226): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(227): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(227): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(227): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(228): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(228): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(228): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(229): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(229): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(229): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(230): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(230): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(230): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(231): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(231): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(231): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(232): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(232): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(232): note: while trying to match the argument list '(char, char16_t)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(233): error C2015: too many characters in constant
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(233): error C2668: 'QString::replace': ambiguous call to overloaded function
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(498): note: could be 'QString &QString::replace(QChar,const QString &,Qt::CaseSensitivity)'
6>C:\dev\Qt\5.11.3\msvc2017_64\include\QtCore/qstring.h(491): note: or       'QString &QString::replace(QChar,QChar,Qt::CaseSensitivity)'
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(233): note: while trying to match the argument list '(char, char16_t)'

Looks like it's unhappy with the superScript.replace() calls. Hopefully there's a simple fix. 😄

justinclift · 2019-03-26T07:37:47Z

Guessing here, but maybe the first argument (eg "0") needs to be a QRegExp() instead?

justinclift · 2019-03-26T09:00:36Z

Yep, with that concept patch (04ebb39), it's compiling with MSVC2017 too. It's probably not exactly the correct regex to use, as it only changes the last digit in the field. So, double digit (10, 11, 50 😉) sort indicators wouldn't work. Feel free to ignore, use it, etc as desired. 😄

@mtissington This is a Win64 build with multi-column sorting + the new sort indicators added:

https://nightlies.sqlitebrowser.org/win64-onceoffs/DB.Browser.for.SQLite-pr1810v1-win64.msi

Wanna try it out? 😄

mtissington · 2019-03-26T17:31:26Z

Oh, yes, this is very good - love the icon too - 😄

MSVC seems to reject the Unicode character literals because they are multibyte (error C2015: too many characters in constant) but seems to work well with the Unicode string literals. See issue #1761 and discussion in PR #1810

mgrojo · 2019-03-26T22:57:42Z

Yep, with that concept patch (04ebb39), it's compiling with MSVC2017 too. It's probably not exactly the correct regex to use, as it only changes the last digit in the field. So, double digit (10, 11, 50 😉) sort indicators wouldn't work. Feel free to ignore, use it, etc as desired. 😄

In fact, it shouldn't be a regexp but a simple character substitution. The problem seems to be in using Unicode character literals. Maybe they aren't supported by MSVC, but your patch suggests that they Unicode character is working inside the string literals, so I've made a new commit for using strings instead of characters and I hope it's working in MSVC too. Could you make a compilation test?

It "felt natural" to then click on the 2nd column again, expecting the sort on that column to be reversed. But no change to it's ordering occurred.

Yes, I think it should change direction too. This is part of the logic whose credit must go to Martin. I only made the visual part and opened this PR so I don't conflict with any possible pending work he might have. @MKleusberg are you still working on this?

In #1761 (comment) @karim describes the way how the multiple clicks on an already added column should behave. In summary, it should be left in the same position and change the ascending or descending direction. Currently it is adding the column again to the sort part of the query, so it only works well when the column you click again is the last added one. I don't know how difficult it would be to change it to behave as Karim says, but it would be ideal.

justinclift · 2019-03-27T03:40:10Z

Could you make a compilation test?

Done. Yep, that worked. MSVC 2017 had no issues this time. 😄

justinclift · 2019-03-27T03:41:32Z

love the icon too

Probably a silly question... er... what icon? 😉

mtissington · 2019-03-27T04:09:22Z

@justinclift sort indicators 👍

MKleusberg · 2019-03-27T10:57:21Z

This looks very good 👍 The only very minor thing I am unsure about is whether to show the little 1 even if there is only one sorting column - as in "if there is a 1 there must be a 2 somewhere too" 😉 Any opinions? But as I said, that seems like a very minor point to me 😄

justinclift · 2019-03-27T11:25:18Z

With a single column selected it doesn't really matter. Though I'd probably show the "1" anyway, just because it will probably hint to our existing users that something has changed and they can click more columns now.

eg "subtle feature education" or something. 😁

mtissington · 2019-03-27T13:59:53Z

i agree, it gives a needed hint.

mgrojo · 2019-03-27T19:31:53Z

I considered leaving the original sort indicator when only one column is selected, but finally went for the homogeneity of always showing the numbers and one of the reasons is that said, about letting the user know that they can select more than one.

Shall we merge this now, then?

justinclift · 2019-03-28T04:07:23Z

Yep, merged. 😄

Kind of thinking the multi-column selection + this to visually show it will be one of the Highlights from the next major (3.12.0) release. Well done! 😄

A second Ctrl+Click over an already added column changes the direction of any column. Before this change, the effect was only applied to the last selected column and a second Ctrl+Click on the other columns triggered a requery without having changed anything. This was mentioned in PR #1810. See also issue #1761.

headerData() now returns column name plus sort indicator in the display role and only the column name in the edit role. This allows to use the edit role for the plot and the copy-with-headers features, so they do not show the sort indicator as part of the column name. See related issue #1409 and PR #1810

justinclift added a commit that referenced this pull request Mar 26, 2019

Potential fix approach for PR #1810

04ebb39

Try to fix compilation with MSVC 2017

b019c4c

MSVC seems to reject the Unicode character literals because they are multibyte (error C2015: too many characters in constant) but seems to work well with the Unicode string literals. See issue #1761 and discussion in PR #1810

justinclift merged commit ea2aa49 into master Mar 28, 2019

justinclift deleted the multi_sort_indicators branch March 28, 2019 04:05

mgrojo restored the multi_sort_indicators branch October 14, 2023 21:27

mgrojo deleted the multi_sort_indicators branch October 14, 2023 21:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Visual sort indicators for multi-column sorting #1810

Visual sort indicators for multi-column sorting #1810

Uh oh!

mgrojo commented Mar 18, 2019

Uh oh!

justinclift commented Mar 26, 2019 •

edited

Loading

Uh oh!

justinclift commented Mar 26, 2019 •

edited

Loading

Uh oh!

justinclift commented Mar 26, 2019

Uh oh!

justinclift commented Mar 26, 2019 •

edited

Loading

Uh oh!

mtissington commented Mar 26, 2019

Uh oh!

mgrojo commented Mar 26, 2019

Uh oh!

justinclift commented Mar 27, 2019

Uh oh!

justinclift commented Mar 27, 2019

Uh oh!

mtissington commented Mar 27, 2019

Uh oh!

MKleusberg commented Mar 27, 2019

Uh oh!

justinclift commented Mar 27, 2019

Uh oh!

mtissington commented Mar 27, 2019

Uh oh!

mgrojo commented Mar 27, 2019

Uh oh!

justinclift commented Mar 28, 2019

Uh oh!

Uh oh!

Uh oh!

Visual sort indicators for multi-column sorting #1810

Visual sort indicators for multi-column sorting #1810

Uh oh!

Conversation

mgrojo commented Mar 18, 2019

Uh oh!

justinclift commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinclift commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinclift commented Mar 26, 2019

Uh oh!

justinclift commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtissington commented Mar 26, 2019

Uh oh!

mgrojo commented Mar 26, 2019

Uh oh!

justinclift commented Mar 27, 2019

Uh oh!

justinclift commented Mar 27, 2019

Uh oh!

mtissington commented Mar 27, 2019

Uh oh!

MKleusberg commented Mar 27, 2019

Uh oh!

justinclift commented Mar 27, 2019

Uh oh!

mtissington commented Mar 27, 2019

Uh oh!

mgrojo commented Mar 27, 2019

Uh oh!

justinclift commented Mar 28, 2019

Uh oh!

Uh oh!

justinclift commented Mar 26, 2019 •

edited

Loading

justinclift commented Mar 26, 2019 •

edited

Loading

justinclift commented Mar 26, 2019 •

edited

Loading