Differential D15747

Remainder dialog for re-enabling the menubar once hidden
ClosedPublic
Actions

Authored by lsartorelli on Sep 25 2018, 1:19 PM.

Details

Reviewers
ngraham
Group Reviewers
VDG
Gwenview
Commits
R260:b0220c132e03: Remainder dialog for re-enabling the menubar once hidden
Summary

BUG: 210620

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
lsartorelli created this revision.Sep 25 2018, 1:19 PM
Restricted Application added a project: Gwenview. · View Herald TranscriptSep 25 2018, 1:19 PM
lsartorelli requested review of this revision.Sep 25 2018, 1:19 PM
broulik added a reviewer: VDG.Sep 25 2018, 2:24 PM
ngraham added a comment.EditedSep 25 2018, 4:07 PM

Thanks! Do we really need to change the parameter for the function? I don't see that showMessage is ever set to false anywhere.

Please change Bug 210620 to BUG: 210620.

lsartorelli updated this revision to Diff 42352.Sep 26 2018, 9:04 AM

Removed unused parameter for toggleMenuBar()

lsartorelli edited the summary of this revision. (Show Details)Sep 26 2018, 9:05 AM
ngraham accepted this revision.Sep 26 2018, 12:54 PM

Looks good to me; any concerns from other members of Gwenview?

This revision is now accepted and ready to land.Sep 26 2018, 12:54 PM
muhlenpfordt added a subscriber: muhlenpfordt.Sep 26 2018, 1:46 PM

If there's no shortcut set (ok, not the default) the message looks a bit strange and doesn't make much sense.
Btw. The title should be called Reminder isn't it?

ngraham added a comment.Sep 26 2018, 2:02 PM

The code for this dialog is lifted straight from Kate: https://cgit.kde.org/kate.git/tree/kate/katemainwindow.cpp?id=a59237913165c12cdeea53d2839618d780561464#n591

Ideally we would have a KStandardAction for this now that it's used in multiple places so that each client doesn't have to re-implement it. I wanted to let @lsartorelli become familiar with this code before suggesting that he create such a thing. :) For now maybe let's be consistent with Kate and then once we have a KStandardAction for this, we can polish up the presentation there so we don't have to duplicate work. Does that sound sane?

muhlenpfordt added a comment.Sep 26 2018, 3:13 PM
In D15747#332180, @ngraham wrote:

For now maybe let's be consistent with Kate and then once we have a KStandardAction for this, we can polish up the presentation there so we don't have to duplicate work. Does that sound sane?

Ok, let's go with this for now and implement a standard way later. ;)

Closed by commit R260:b0220c132e03: Remainder dialog for re-enabling the menubar once hidden (authored by lsartorelli, committed by ngraham). · Explain WhySep 26 2018, 10:50 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mapp/mainwindow.cpp (6 lines)

Diff 42352

View Options

app/mainwindow.cpp

Loading...