New design for extended search options
ClosedPublic

Authored by iasensio on Oct 12 2019, 11:13 PM.

Details

Summary

Replace the arrays of radiobuttons in dolphin search box to more simple dropdown boxes.
This provides a leaner look, saving a lot user space on the main view and it is more consistent with the 'Search tools' in the most known sites (Google, DuckDuckGo, etc.)
There is room for improvement, as QComboBox doesn't match perfectly with the tool buttons used avobe, but I think it is an improvement over the current situation.

BEFORE:

AFTER:

Test Plan

Same behavior with different aesthetics

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
iasensio created this revision.Oct 12 2019, 11:13 PM
Restricted Application added a project: Dolphin. Β· View Herald TranscriptOct 12 2019, 11:13 PM
Restricted Application added a subscriber: kfm-devel. Β· View Herald Transcript
iasensio requested review of this revision.Oct 12 2019, 11:13 PM
iasensio updated this revision to Diff 67821.Oct 13 2019, 12:29 AM
  • Validate terms before applying

This is a lot nicer. I think we could even get rid of the More Options/Fewer Options toggle button and just always show the search options now that they don't take up much space.

ndavis added a subscriber: ndavis.Oct 13 2019, 6:13 AM

Why do the comboboxes look so thin in the video?

I think we could even get rid of the More Options/Fewer Options toggle button and just always show the search options now that they don't take up much space.

I had also the same feeling, but I would like to address/discuss that later on when/if this (and some companion patches) get merged πŸ˜ƒ

Why do the comboboxes look so thin in the video?

It is the standard height for a QComboBox with no frame. I think it picks an implicit minimum height from the selected items.
I would like to match the height of the avobe QToolButtons, but I may need some help with that, as to what is the best way to know that height and then to set it.
Anyway I'm going to play around with the layout and minimumSizeand see if I can come with something nicer.

iasensio updated this revision to Diff 67838.Oct 13 2019, 12:28 PM
  • Fix combos' height

Why do the comboboxes look so thin in the video?

Don't mind my last comment. It was easier to fix than expected:

This is so nice!

iasensio updated this revision to Diff 68346.Sun, Oct 20, 10:40 AM
  • Rebase to master
elvisangelaccio requested changes to this revision.Sun, Oct 20, 11:25 AM

Just minor issues, code and UI change look good otherwise :)

src/search/dolphinfacetswidget.cpp
122

Unrelated change, please move it to another commit.

140

Unrelated change, please move it to another commit.

(and subTerms should be made const to avoid a detach).

155–159

Coding style: no space after/before ()

165–166

Coding style: add space before/after comparison operators.

177

Unrelated change, please move it to another commit.

This revision now requires changes to proceed.Sun, Oct 20, 11:25 AM

Fails to compile for me, btw:

./src/search/dolphinfacetswidget.cpp:86:5: error: use of undeclared identifier 'm_anyType'
    m_anyType->setChecked(true);
    ^
../src/search/dolphinfacetswidget.cpp:87:5: error: use of undeclared identifier 'm_anytime'
    m_anytime->setChecked(true);
    ^
../src/search/dolphinfacetswidget.cpp:88:5: error: use of undeclared identifier 'm_anyRating'
    m_anyRating->setChecked(true);
    ^
3 errors generated.
iasensio updated this revision to Diff 68355.Sun, Oct 20, 11:44 AM
  • Fix rebasing

Fails to compile for me, btw:

./src/search/dolphinfacetswidget.cpp:86:5: error: use of undeclared identifier 'm_anyType'
    m_anyType->setChecked(true);
    ^
../src/search/dolphinfacetswidget.cpp:87:5: error: use of undeclared identifier 'm_anytime'
    m_anytime->setChecked(true);
    ^
../src/search/dolphinfacetswidget.cpp:88:5: error: use of undeclared identifier 'm_anyRating'
    m_anyRating->setChecked(true);
    ^
3 errors generated.

Sorry, failed to update that when rebasing. It should be fixed now.
I'm addressing your comments in the next update.

iasensio updated this revision to Diff 68358.Sun, Oct 20, 12:01 PM
iasensio marked 5 inline comments as done.
  • Remove unrelated changes
  • Coding style
elvisangelaccio accepted this revision.Sun, Oct 27, 9:59 PM

I assume the VDG is fine with this even without formal approval ;)

This revision is now accepted and ready to land.Sun, Oct 27, 9:59 PM
This revision was automatically updated to reflect the committed changes.

You would be correct. :)

@iasensio Now that this is in, I would support removing the button that shows and hides the expended view and just always show it. That will probably be sufficient to fix https://bugs.kde.org/show_bug.cgi?id=386754.

@iasensio Now that this is in, I would support removing the button that shows and hides the expended view and just always show it. That will probably be sufficient to fix https://bugs.kde.org/show_bug.cgi?id=386754.

Nice. I had it already on my to-do list. I also suffer from long translated actions πŸ˜ƒ