D24263: edit mode action
This revision was automatically updated to reflect the committed changes. Closed by commit R242:acbb200ac2cc: edit mode action (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D24263?vs=68224=68422#toc REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24263?vs=68224=68422 REVISION DETAIL https://phabricator.kde.org/D24263 AFFECTED FILES src/plasma/corona.cpp src/plasma/corona.h To: mart, #plasma, #vdg, ngraham, davidedmundson Cc: davidedmundson, broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
davidedmundson accepted this revision. REPOSITORY R242 Plasma Framework (Library) BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma, #vdg, ngraham, davidedmundson Cc: davidedmundson, broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart updated this revision to Diff 68224. mart added a comment. - text is always Customize Layout... REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24263?vs=67896=68224 BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 AFFECTED FILES src/plasma/corona.cpp src/plasma/corona.h To: mart, #plasma, #vdg, ngraham Cc: davidedmundson, broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
davidedmundson added a comment. GB_2's comment needs fixing, two strings are out of sync. Other than that, ship it. INLINE COMMENTS > GB_2 wrote in corona.cpp:502 > Missing ellipsis (...) 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. REPOSITORY R242 Plasma Framework (Library) BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma, #vdg, ngraham Cc: davidedmundson, broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
GB_2 added inline comments.
INLINE COMMENTS
> corona.cpp:502
> +});
> +editAction->setText(i18n("Customize Layout"));
> +editAction->setAutoRepeat(true);
Missing ellipsis (...)
REPOSITORY
R242 Plasma Framework (Library)
BRANCH
phab/editModeActionwq!
REVISION DETAIL
https://phabricator.kde.org/D24263
To: mart, #plasma, #vdg, ngraham
Cc: broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart marked an inline comment as done. REPOSITORY R242 Plasma Framework (Library) BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma, #vdg, ngraham Cc: broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart updated this revision to Diff 67896. mart added a comment. - add ellipsis REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24263?vs=67557=67896 BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 AFFECTED FILES src/plasma/corona.cpp src/plasma/corona.h To: mart, #plasma, #vdg, ngraham Cc: broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
GB_2 added inline comments.
INLINE COMMENTS
> corona.cpp:419
> +} else {
> +editAction->setText(i18n("Customize Layout"));
> +}
Missing ellipsis (...)
REPOSITORY
R242 Plasma Framework (Library)
BRANCH
phab/editModeActionwq!
REVISION DETAIL
https://phabricator.kde.org/D24263
To: mart, #plasma, #vdg, ngraham
Cc: broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
ngraham accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma, #vdg, ngraham Cc: broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart added inline comments. INLINE COMMENTS > broulik wrote in corona.cpp:378 > Why is this switch to explicitly stating all the cases? > The action is always visible and enabled only when Mutable ah, it wanted to be setVisible(false) in case of systemimmutable, so they should be all different REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma, #vdg Cc: broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart updated this revision to Diff 67557. mart added a comment. - add parameter to editModeChanged - hide action on systemimmutable REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24263?vs=67113=67557 BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 AFFECTED FILES src/plasma/corona.cpp src/plasma/corona.h To: mart, #plasma, #vdg Cc: broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
broulik added inline comments.
INLINE COMMENTS
> corona.cpp:378
> +switch (d->immutability) {
> +case Types::UserImmutable:
> +action->setEnabled(false);
Why is this switch to explicitly stating all the cases?
The action is always visible and enabled only when Mutable
> corona.cpp:379
> +case Types::UserImmutable:
> +action->setEnabled(false);
> +action->setVisible(true);
fall-through?
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D24263
To: mart, #plasma, #vdg
Cc: broulik, GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
GB_2 resigned from this revision. This revision now requires review to proceed. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma, #vdg Cc: GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
GB_2 accepted this revision as: VDG, GB_2. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma, #vdg, GB_2 Cc: GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
GB_2 added a reviewer: VDG. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma, #vdg Cc: GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
ngraham added a comment. "Customize Layout" is fine IMO. :) REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma Cc: GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart updated this revision to Diff 67113. mart added a comment. - Customize Layout REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24263?vs=67051=67113 BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 AFFECTED FILES src/plasma/corona.cpp src/plasma/corona.h To: mart, #plasma Cc: GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart added a comment. 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 ;) REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma Cc: GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart added a comment. the last, "the least the name "edit mode" is user facing, the better it is REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma Cc: GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
GB_2 added inline comments. INLINE COMMENTS > ngraham wrote in corona.cpp:406 > 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 What about... - "Switch to Edit Mode"/"Leave Edit Mode" - "Enter Edit Mode"/"Exit Edit Mode" - "Edit Panels and Widgets"/"Finish Editing Panels and Widgets" REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma Cc: GB_2, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns
D24263: edit mode action
mart updated this revision to Diff 67051. mart added a comment. - always keep the edit action consistent REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24263?vs=66953=67051 BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 AFFECTED FILES src/plasma/corona.cpp src/plasma/corona.h To: mart, #plasma Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
D24263: edit mode action
ngraham added inline comments.
INLINE COMMENTS
> corona.cpp:406
> +} else {
> +editAction->setText(i18n("Edit Widgets"));
> +}
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
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D24263
To: mart, #plasma
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
D24263: edit mode action
ngraham added a dependent revision: D24265: support global edit mode. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24263: edit mode action
ngraham added tasks: T10190: (Re)define modes when editing panels and widgets, T11094: Improve discoverability of widget configuration, T10402: The Desktop Toolbox. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D24263 To: mart, #plasma Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D24263: edit mode action
mart created this revision. mart added a reviewer: Plasma. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. mart requested review of this revision. REVISION SUMMARY needed for the context menu entry TEST PLAN with the p-w- portion the action shows in the context menu REPOSITORY R242 Plasma Framework (Library) BRANCH phab/editModeActionwq! REVISION DETAIL https://phabricator.kde.org/D24263 AFFECTED FILES src/plasma/corona.cpp src/plasma/corona.h To: mart, #plasma Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
