Differential D20471

Add "What's This?" to nearly everything in the main window
ClosedPublic
Actions

Authored by felixernst on Apr 11 2019, 3:44 PM.

Details

Reviewers
elvisangelaccio
Group Reviewers
Dolphin
Commits
R318:bd788a3ff425: Add "What's This?" to nearly everything in the main window
Summary

This commit adds "What's This?" help to nearly everything in the
Dolphin main window (panels, views, buttons, ...). It adds the "?"
to the title bar so this help can easily be called.

For links in those help texts to work the WhatsThisClickedEvents are
handled in the main window class. This doesn't work for menus because
events from them aren't forwarded to the main window for some
reason so EventFilters are installed for the Control button menus.

Modifying the "Help" menu of KXmlGui is deprecated so no EventFilter
can be installed in the menubar. Therefore help texts without links
are provided for the menubar.

Test Plan

Check if the event handling might make any problems.
Check for any big mistakes in the help messages.

Diff Detail

Repository
R318 Dolphin
Lint
Lint Skipped
Unit
Unit Tests Skipped
felixernst created this revision.Apr 11 2019, 3:44 PM
Restricted Application added a subscriber: kfm-devel. · View Herald TranscriptApr 11 2019, 3:44 PM
felixernst requested review of this revision.Apr 11 2019, 3:44 PM
yurchor added a subscriber: yurchor.Apr 11 2019, 5:54 PM

Thanks for fixing these typos.

src/dolphinmainwindow.cpp
1433

Typo: informations -> information

1438

Typo: informations -> information

felixernst updated this revision to Diff 56177.EditedApr 14 2019, 9:47 AM

Fix typos: informations -> information

felixernst marked 2 inline comments as done.May 14 2019, 9:24 AM

Bump

felixernst edited the test plan for this revision. (Show Details)May 14 2019, 9:36 AM
elvisangelaccio added a subscriber: elvisangelaccio.May 19 2019, 9:44 AM

The patch does not apply, can you please rebase it?

elvisangelaccio requested changes to this revision.May 19 2019, 10:09 AM
elvisangelaccio added inline comments.
src/dolphinmainwindow.cpp
82–83

Please keep the includes sorted by name.

1155

Please add const

1161

Please call it cutAction. (I know we use paste below for the paste action, but that's old code).

1167

Please call it copyAction.

1182

Please call it findAction.

1191

Please call it selectAllAction.

1427

Please add const

1912

Please add const

1928

Funny name :D

Please add const here as well

1950

Please add const

1962

Please add const

1976

Please add const

1984

Please add const

1991

Please add const

2003–2009

Coding style: please use

if (event->type() == QEvent::WhatsThisClicked) {

}
2014

Unnecessary semicolon.

2015–2021

Coding style (same as above).

This revision now requires changes to proceed.May 19 2019, 10:09 AM
felixernst updated this revision to Diff 58357.May 20 2019, 12:57 PM
felixernst marked 17 inline comments as done.

Rebased to current master
Applied all the requested changes

(Having a bit of trouble with my dev environment right now
but I am moderately confident everything's alright.)

elvisangelaccio added a comment.May 25 2019, 3:20 PM

Patch looks almost ready now.

Can you expand a bit on

... Modifying the "Help" menu of KXmlGui is deprecated so no EventFilter can be installed in the menubar.

? Is this deprecation documented somewhere?

src/dolphinmainwindow.cpp
1876–1878

This is not correct, as the Control button only has a subset of all the available actions.

felixernst added a comment.May 27 2019, 12:19 PM
This comment was removed by felixernst.
felixernst added a comment.EditedMay 27 2019, 12:55 PM

Patch looks almost ready now.

I'm happy to hear that!

? Is this deprecation documented somewhere?

Receiving the HelpMenu is deprecated in KMainWindow:
KMainWindow::customHelpMenu (https://api.kde.org/frameworks/kxmlgui/html/classKMainWindow.html#aff97bcd122771b99b699a4837b728907)
KMainWindow::helpMenu (https://api.kde.org/frameworks/kxmlgui/html/classKMainWindow.html#a669da563f6d8e85e6f590980593353b4)

I was trying for days to make the links within the "What's This?"-help within the menus of the menubar react to WhatsThisClickedEvents. I couldn't find a way to install EventFilters to any menu in the menubar. This was only really a problem for the help menu because all but one link I wanted to add was there.
I think I read somewhere that it was deprecated because all KDE applications should have the exact same help menu.
The best solution might be either:

  • to add event handling of WhatsThisClickedEvents to all menus of the menubars of all KDE applications in a way that all links work. Probably somewhere in the class KMainWindow or KXMLGui.
  • to forward the events from the menubar so they can be handled in an application-specific manner.
felixernst updated this revision to Diff 58722.EditedMay 27 2019, 1:10 PM
felixernst marked an inline comment as done.

Change "all its contents" to "most of its contents"

broulik added a subscriber: broulik.May 27 2019, 1:33 PM

You might want to add the window flag Qt::WindowContextHelpButtonHint to all windows that use what's this. Qt will stop automatically adding the button for dialogs in Qt 6.

felixernst added a comment.EditedMay 27 2019, 2:12 PM

I was already thinking about working on explicitly setting or not setting this flag at the right places. In particular I was thinking about dynamically changing this flag depending on the page in the system settings one is currently viewing so users don't get frustrated when no help is available for the current one.
Though concerning Dolphin the only windows that use "What's This?" seem to be Settings/Configure Shortcuts... and Help/Report Bug... and those aren't specific to Dolphin.

I am a bit hesitant to put work into the '?' in the titlebar because I read here T9986: Delete "What's This" inline help functionality that Wayland won't be able to display it. But maybe that is subject to change. I'll think about it.

elvisangelaccio accepted this revision.Jun 16 2019, 7:57 PM
elvisangelaccio added inline comments.
src/dolphinmainwindow.cpp
2034

Please add a space before this function.

This revision is now accepted and ready to land.Jun 16 2019, 7:57 PM
felixernst updated this revision to Diff 60152.Jun 20 2019, 4:39 PM
felixernst marked an inline comment as done.

Unify sentence structure and add requested space
Sentences should start with "This does/is/etc." for
better intelligibility if possible instead of using
the imperative or omitting the subject.

felixernst updated this revision to Diff 60159.Jun 20 2019, 5:02 PM

Start help message for "Show hidden places" with "This"

felixernst added a comment.EditedJun 20 2019, 5:06 PM

Excuse me for not making these small changes and sentence restructuring sooner. Re-reading my help messages now brought different details to my attention.

elvisangelaccio accepted this revision.Jun 23 2019, 2:18 PM
felixernst added a comment.Jun 23 2019, 4:28 PM

I don't have commit access. Can you land the patch for me, per favore? :)

elvisangelaccio requested changes to this revision.Jun 23 2019, 5:09 PM

Sorry but there are a lot of trailing whitespace changes (which unfortunately phabricator ignores).

Can you please fix them? (use git diff locally to detect them).

This revision now requires changes to proceed.Jun 23 2019, 5:09 PM
felixernst updated this revision to Diff 60530.Jun 23 2019, 6:30 PM

Remove trailing whitespaces

elvisangelaccio accepted this revision.Jun 23 2019, 7:24 PM
This revision is now accepted and ready to land.Jun 23 2019, 7:24 PM
Closed by commit R318:bd788a3ff425: Add "What's This?" to nearly everything in the main window (authored by felixernst, committed by elvisangelaccio). · Explain WhyJun 23 2019, 7:25 PM
This revision was automatically updated to reflect the committed changes.
broulik added inline comments.Jun 25 2019, 1:25 PM
src/dolphinmainwindow.cpp
1474

Crashes here for me. The action looks like it's only added when you have #if HAVE_BALOO

felixernst added a comment.Jun 25 2019, 1:38 PM

Thanks for fixing this!

Revision Contents
Changeset List

Diff 56177

View Options

src/dolphinmainwindow.h

Loading...
View Options

src/dolphinmainwindow.cpp

Loading...
View Options

src/dolphinviewcontainer.cpp

Loading...
View Options

src/statusbar/dolphinstatusbar.cpp

Loading...
View Options

src/views/dolphinviewactionhandler.h

Loading...
View Options

src/views/dolphinviewactionhandler.cpp

Loading...