Details
- Reviewers
ngraham elvisangelaccio - Group Reviewers
Dolphin VDG - Maniphest Tasks
- T10258: Use correct search bars and use ellipsis whenever needed to follow the KDE HIG
- Commits
- R318:8dc5c7a199ae: Use placeholder for search action
Compile and run
Diff Detail
- Repository
- R318 Dolphin
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 9717 Build 9735: arc lint + arc unit
src/search/dolphinsearchbox.cpp | ||
---|---|---|
362 | Should it be "Search..." or even "Search…" (GNOME has already switched to the Unicode ellipsis)? Thanks in advance for your answer. |
src/search/dolphinsearchbox.cpp | ||
---|---|---|
362 | Unicode ellipsis are better thanks ;) |
The menu item and the toolbar button say "Find". "Search" is really the correct term here, so we should change the text of the menu item and toolbar button to match it.
Interesting Dolphin uses the KStandardAction::Find enum (https://api.kde.org/frameworks/kconfigwidgets/html/namespaceKStandardAction.html#aa71041e1fa0a0c740e00b423ae684334a70ba4f3d9ae876b26883ea355627d236) . Should I create a non-standard action instead?
You should be able to override its text with something like action->setText(i18n("foo bar baz"));
Then it should be changed in kconfig/kconfigwidgets. But at least Firefox uses the term"Find" when I press CTRL+F, so I don't know...
src/search/dolphinsearchbox.cpp | ||
---|---|---|
362 | This will not be translatable, please read https://api.kde.org/frameworks/ki18n/html/prg_guide.html |
Hmm, the menu and toolbar items still say "Find..."
src/search/dolphinsearchbox.cpp | ||
---|---|---|
362 | I think we agreed in the VDG chat to use three periods instead of the actual ellipsis character for now. |
- Use 3 dots instead of unicode
- Apply same change to filter bar
- This time change Find to Search correctly
Now Dolphin doesn't compile:
/home/nate/kde/src/dolphin/src/dolphinmainwindow.cpp: In member function ‘void DolphinMainWindow::setupActions()’: /home/nate/kde/src/dolphin/src/dolphinmainwindow.cpp:1145:5: error: invalid use of member function ‘void DolphinMainWindow::find()’ (did you forget the ‘()’ ?) find->setText(i18n("Search...")); ^~~~ /home/nate/kde/src/dolphin/src/dolphinmainwindow.cpp:1145:9: error: base operand of ‘->’ is not a pointer find->setText(i18n("Search...")); ^~ /home/nate/kde/src/dolphin/src/dolphinmainwindow.cpp:1144:14: warning: unused variable ‘search’ [-Wunused-variable] QAction *search = KStandardAction::find(this, &DolphinMainWindow::find, actionCollection()); ^~~~~~
Looks like that should be search->setText(blablabla)
Make sure you're testing your changes! :)
I still think that the rename from "Search" to "Find" should be done either upstream/globally or not at all.
Anyway, it's unrelated from this commit, which is just about moving to placeholders everywhere (which I agree with).
src/dolphinmainwindow.cpp | ||
---|---|---|
931–933 | We shouldn't rename a standard action. From System Settings -> Global Shortcuts it will still be named "Find..." |
Search and Find have different meanings because they do different things: locating files or items that match the given text in a data set of indeterminate or unclear size is "Search"; locating instances of text within the open document or page is "Find". But they should both have the same common ctrl+F keyboard shortcut. So it cannot be done globally.
Any idea how we move forward with this?
Fair enough. Another thing I didn't consider is that the menubar is hidden by default, so most dolphin users probably never even see the text of the search action.
@ognarb Please rename the variables as mentioned inline and then we can ship it.
src/dolphinmainwindow.cpp | ||
---|---|---|
932 | Please call this variable searchAction. | |
1144 | And this one too. |
JFYI it's generally considered polite to wait until all reviewers with an outstanding "Changes Requested" status to change it to "Accepted" before committing. :)