edit mode action
ClosedPublic

Authored by mart on Sep 27 2019, 2:48 PM.

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
phab/editModeActionwq!
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17851
Build 17869: arc lint + arc unit
mart created this revision.Sep 27 2019, 2:48 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 27 2019, 2:48 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
mart requested review of this revision.Sep 27 2019, 2:48 PM
ngraham added inline comments.
src/plasma/corona.cpp
419

This isn't just editing editing widgets; maybe "Enter Edit Mode" ? That would make the name better correspond with the text of the opposite state too.

Alternatives:

  • Edit Layout and Widgets/Freeze Layout and Widgets
  • Configure Layout and Widgets/Freeze Layout and Widgets
  • Configure Layout and Widgets/Finish Configuring Layout and Widgets
  • Customize Layout/Finish Customizing Layout
mart updated this revision to Diff 67051.Sep 30 2019, 9:00 AM
  • always keep the edit action consistent
GB_2 added a subscriber: GB_2.Sep 30 2019, 2:24 PM
GB_2 added inline comments.
src/plasma/corona.cpp
419

What about...

  • "Switch to Edit Mode"/"Leave Edit Mode"
  • "Enter Edit Mode"/"Exit Edit Mode"
  • "Edit Panels and Widgets"/"Finish Editing Panels and Widgets"
mart added a comment.Oct 1 2019, 9:10 AM

the last, "the least the name "edit mode" is user facing, the better it is

mart added a comment.Oct 1 2019, 9:14 AM

i think in the end
Customize Layout/Finish Customizing Layout

i prefer not to name the term "mode" at all if i can, both because is kinda a nerdy term and because modes are an ux cardinal sin, so at least not being too proud of it ;)

mart updated this revision to Diff 67113.Oct 1 2019, 9:17 AM
  • Customize Layout

"Customize Layout" is fine IMO. :)

GB_2 accepted this revision as: VDG, GB_2.Oct 3 2019, 11:39 AM
GB_2 added a reviewer: VDG.
This revision is now accepted and ready to land.Oct 3 2019, 11:39 AM
GB_2 resigned from this revision.Oct 3 2019, 11:53 AM
This revision now requires review to proceed.Oct 3 2019, 11:53 AM
broulik added a subscriber: broulik.Oct 9 2019, 1:21 PM
broulik added inline comments.
src/plasma/corona.cpp
378

Why is this switch to explicitly stating all the cases?
The action is always visible and enabled only when Mutable

379

fall-through?

mart updated this revision to Diff 67557.Oct 9 2019, 2:33 PM
  • add parameter to editModeChanged
  • hide action on systemimmutable
mart added inline comments.Oct 9 2019, 3:12 PM
src/plasma/corona.cpp
378

ah, it wanted to be setVisible(false) in case of systemimmutable, so they should be all different

ngraham accepted this revision.Oct 11 2019, 3:27 PM
This revision is now accepted and ready to land.Oct 11 2019, 3:27 PM
GB_2 added inline comments.Oct 12 2019, 2:08 PM
src/plasma/corona.cpp
419

Missing ellipsis (...)

mart updated this revision to Diff 67896.Oct 14 2019, 12:44 PM
  • add ellipsis
mart marked an inline comment as done.Oct 14 2019, 12:45 PM
GB_2 added inline comments.Oct 18 2019, 8:37 AM
src/plasma/corona.cpp
502

Missing ellipsis (...)

GB_2's comment needs fixing, two strings are out of sync.

Other than that, ship it.

src/plasma/corona.cpp
502

Given how easy it is to get these out of sync, can I suggest removing all the code to update editAction's text from Corona::setEditMode and put it in a connect here. It'll keep the editAction logic altogether.

mart updated this revision to Diff 68224.Oct 18 2019, 10:56 AM
  • text is always Customize Layout...
davidedmundson accepted this revision.Oct 18 2019, 11:00 AM
This revision was automatically updated to reflect the committed changes.