D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu abandoned this revision.
liushuyu added a comment.


  This patch has been migrated to 
https://invent.kde.org/plasma/kscreen/-/merge_requests/1

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D25539

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu added a comment.


  Any other suggestions for this patch?

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D25539

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu updated this revision to Diff 70474.
liushuyu added a comment.


  Address ndavis' comments

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25539?vs=70451=70474

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25539

AFFECTED FILES
  kcm/config_handler.cpp
  kcm/config_handler.h
  kcm/kcm.cpp
  kcm/kcm.h
  kcm/package/contents/ui/main.qml

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu added a comment.


  In D25539#568678 , @ndavis wrote:
  
  > Is there a way to tell the revert timer not to be used if the user only 
makes a change that requires a session restart? It wouldn't be very useful to 
ask a user to confirm if the screen is displaying correctly if the change can't 
be seen.
  
  
  Sorry, I don't know how to determine whether the changes only require a 
session restart properly. To my knowledge, the global scale option is the only 
setting that probably does not need the timer and my current idea is to set a 
boolean flag somewhere to indicate the global scale has changed and see if this 
is the only change user has made.

INLINE COMMENTS

> ndavis wrote in main.qml:109
> Since you are keeping the recently applied configuration rather than applying 
> a new one, wouldn't it be better to say something like "Keep"? It would also 
> be better to use `dialog-ok-apply` as the icon for consistency with similar 
> controls.

Will do

> ndavis wrote in main.qml:115
> I would put Undo to the left of Keep/Apply. Normally, Undo/Reset is to the 
> far left of Apply. That can't be the case here, but I think preserving the 
> order is better for muscle memory.

I will address this

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D25539

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu added a comment.


  After applying the patches from 
https://cgit.kde.org/kirigami.git/commit/?id=8c1e5b1336e6dcae0a1b7756977fcd9368f853a2,
  it now looks like this:
  
  F7787159: image.png 

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D25539

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu updated this revision to Diff 70451.
liushuyu added a comment.


  Update icons and stop the timer on actions pressed

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25539?vs=70390=70451

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25539

AFFECTED FILES
  kcm/config_handler.cpp
  kcm/config_handler.h
  kcm/kcm.cpp
  kcm/kcm.h
  kcm/package/contents/ui/main.qml

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu marked 2 inline comments as done.
liushuyu added a comment.


  Now it looks like this
  F7785973: image.png 

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D25539

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu marked 2 inline comments as done.
liushuyu added inline comments.

INLINE COMMENTS

> ngraham wrote in main.qml:32
> Doesn't seem to be used; the timer duration is hardcoded on the C++ side

Now, it's used by the Timer

> ngraham wrote in main.qml:110
> I would change these to `StandardButton.Apply` and `StandardButton.Discard`

Now InlineMessage is used

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D25539

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: feat(kcm): add revert timer

[Zixing Liu]

liushuyu updated this revision to Diff 70390.
liushuyu added a comment.


  Use InlineMessage instead of MessageBox and move the timer to the QML portion

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25539?vs=70329=70390

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25539

AFFECTED FILES
  kcm/config_handler.cpp
  kcm/config_handler.h
  kcm/kcm.cpp
  kcm/kcm.h
  kcm/package/contents/ui/main.qml

To: liushuyu, #vdg, #plasma, romangg
Cc: ngraham, broulik, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: KScreen KCM: Add a revert timer to the settings page

[Zixing Liu]

liushuyu updated this revision to Diff 70329.
liushuyu added a comment.


  Fix the diff

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25539?vs=70328=70329

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25539

AFFECTED FILES
  kcm/config_handler.cpp
  kcm/config_handler.h
  kcm/kcm.cpp
  kcm/kcm.h
  kcm/package/contents/ui/main.qml

To: liushuyu, #vdg, #plasma
Cc: ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25539: KScreen KCM: Add a revert timer to the settings page

[Zixing Liu]

liushuyu created this revision.
liushuyu added a reviewer: VDG.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
liushuyu requested review of this revision.

REVISION SUMMARY
  Add a revert timer and an option for the user to revert the settings when 
they accidentally messed up the settings and unable to see the screen.
  
  The current implementation is very rudimentary, a message box is shown and 
the text is static (no countdown in the dialog box).
  
  F7784367: 2019-11-25_20-46.png 
  
  F7784366: 2019-11-25_20-46_1.png 

TEST PLAN
  1. Open the System Settings and navigate to the "Display and Monitor" -> 
"Display Configuration."
  2. Change any settings and hit Apply.
  3. Don't click on the pop-up, wait for 10 seconds and see if the changes will 
be reverted and the dialog box closed.
  4. Repeat step 1 to step 2 and this time, click "cancel" and see if the 
changes will be reverted and the dialog box closed.
  5. Repeat step 1 to step 2 but this time click "save" and see if the changes 
will be saved and the dialog box closed, wait for 10 seconds to see if the 
changes are still retained.

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D25539

AFFECTED FILES
  kcm/config_handler.cpp
  kcm/config_handler.h
  kcm/kcm.cpp
  kcm/kcm.h
  kcm/package/contents/ui/main.qml

To: liushuyu, #vdg
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart