Differential D19904

Prompt user to save/discard changes upon closing config dialog
ClosedPublic
Actions

Authored by amhndu on Mar 20 2019, 9:35 AM.

Details

Reviewers
elvisangelaccio
Group Reviewers
Dolphin
Commits
R318:017cd2322a0a: Prompt user to save/discard changes upon closing config dialog
Summary

When the configuration dialog is closed with unsaved changes,
a message box is prompted to save/discard them or cancel the event.

BUG: 391206

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.
amhndu created this revision.Mar 20 2019, 9:35 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptMar 20 2019, 9:35 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
amhndu requested review of this revision.Mar 20 2019, 9:35 AM
amhndu edited the summary of this revision. (Show Details)Mar 20 2019, 9:44 AM
elvisangelaccio added a subscriber: elvisangelaccio.Mar 20 2019, 8:48 PM

What if the user press Cancel instead of closing the window?

amhndu added a comment.Mar 21 2019, 6:16 AM

Should that really prompt the user? I expect clicking Cancel to close the dialog

ngraham added a subscriber: ngraham.Mar 24 2019, 4:26 PM
In D19904#435546, @amhndu wrote:

Should that really prompt the user? I expect clicking Cancel to close the dialog

I would tend to agree. "Cancel" is very much a "discard everything, I didn't mean to change stuff" action.

elvisangelaccio requested changes to this revision.Mar 24 2019, 5:36 PM
elvisangelaccio added inline comments.
src/settings/dolphinsettingsdialog.cpp
163

Please use semantic markup and replace \n with <nl/>.

See https://api.kde.org/frameworks/ki18n/html/prg_guide.html#kuit_markup for more details.

src/settings/dolphinsettingsdialog.h
52

QWidget::closeEvent() is protected, I'd make it protected also here.

54

There is already a private section in this class.

This revision now requires changes to proceed.Mar 24 2019, 5:36 PM
ngraham added inline comments.Mar 24 2019, 5:38 PM
src/settings/dolphinsettingsdialog.cpp
163

And really, do we even need a line break at all here?

amhndu updated this revision to Diff 54757.Mar 25 2019, 10:12 AM
  • Update with requested changes
amhndu marked 4 inline comments as done.Mar 25 2019, 10:13 AM
elvisangelaccio accepted this revision.Mar 25 2019, 8:55 PM

Please push to master only.

This revision is now accepted and ready to land.Mar 25 2019, 8:55 PM
Closed by commit R318:017cd2322a0a: Prompt user to save/discard changes upon closing config dialog (authored by amhndu). · Explain WhyMar 26 2019, 8:54 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 54836

View Options

src/settings/dolphinsettingsdialog.h

Loading...
View Options

src/settings/dolphinsettingsdialog.cpp

Loading...