[comic] Modernize configuration windows
ClosedPublic

Authored by filipf on Mar 16 2019, 10:14 AM.

Details

Summary

All 3 comic's configuration windows (General, Appearance, Advanced) have been ported to QQC2 and Kirigami.FormLayout. Other changes include:

  • the icon for the Appearance category is now consistent this with the icon used for this category elsewhere
  • improving the wording of options
  • removing the sections in Appearance and Advanced tabs
  • making it explicit that the navigation buttons can always be shown or only on hover
Test Plan

Before (General):

After (General):

Before (Appearance):

After (Appearance):

Before (Advanced):

After (Advanced):

Diff Detail

Repository
R114 Plasma Addons
Branch
modernized-comic-settings (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9659
Build 9677: arc lint + arc unit
filipf created this revision.Mar 16 2019, 10:14 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 16 2019, 10:14 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
filipf requested review of this revision.Mar 16 2019, 10:14 AM
filipf edited the summary of this revision. (Show Details)Mar 16 2019, 10:21 AM
filipf edited the test plan for this revision. (Show Details)
filipf added reviewers: rooty, ngraham, VDG, Plasma.
filipf edited the test plan for this revision. (Show Details)
filipf updated this revision to Diff 54000.Mar 16 2019, 10:28 AM
filipf edited the summary of this revision. (Show Details)
filipf edited the test plan for this revision. (Show Details)

when value for minutes is 1 use "min" instead of "mins", when value for days is 1 use "day"
instead of "days"

filipf updated this revision to Diff 54001.Mar 16 2019, 10:31 AM

1 strips per comic -> 1 strip per comic

ngraham requested changes to this revision.Mar 16 2019, 10:37 AM
ngraham added inline comments.
applets/comic/package/contents/ui/configAdvanced.qml
55

The way you do this properly is as follows, with i18ncp():

text: i18ncp("@item:valuesuffix spacing to number + unit", " strip per comic", "strips per comic")
applets/comic/package/contents/ui/configAppearance.qml
55

Might work better with just "Show:" (and then provide some context for translators)

applets/comic/package/contents/ui/configGeneral.qml
59

If you used a header rather than a left label to work around the fact that the repeater underneath it makes this hard, you can check out how I made that work in the clock settings' plugins view: https://cgit.kde.org/plasma-workspace.git/tree/applets/digital-clock/package/contents/ui/configCalendar.qml#n59

88

This could be shortened:

"Middle-click on comic to display at original size"

97

I'd recommend putting these in the FormLayout rather than using a RowLayout, since now the spinboxes aren't aligned anymore.

110

Same; use i18ncp() rather than synthesizing different i18n() calls for the singular and plural versions (also I think you're missing the i18n() calls themselves)

127

Ditto

This revision now requires changes to proceed.Mar 16 2019, 10:37 AM

Definitely some silly mistakes in there, thanks for showing me how to do it right!

As for the looks, I'm struggling with liking the placement of the headings when Kirigami.FormData.label is used, hence the row layouts. Case in point:

The gaps in vertical spaces the headings leave just doesn't look good to me.

ngraham added a subscriber: mart.Mar 16 2019, 5:24 PM

Definitely some silly mistakes in there, thanks for showing me how to do it right!

As for the looks, I'm struggling with liking the placement of the headings when Kirigami.FormData.label is used, hence the row layouts. Case in point:

The gaps in vertical spaces the headings leave just doesn't look good to me.

Me neither.

But that's "as designed" and we shouldn't work around the design; we should fix it if we think it's ugly.

Paging Dr. @mart :)

filipf updated this revision to Diff 54254.Mar 18 2019, 7:17 PM

avoid using headings, fix spinbox strings

filipf edited the test plan for this revision. (Show Details)Mar 18 2019, 7:18 PM
filipf marked 7 inline comments as done.Mar 18 2019, 7:20 PM
filipf added inline comments.
applets/comic/package/contents/ui/configAdvanced.qml
55

It's always the plural now, but I also realized we shouldn't be using singular if value is 1 because some Slavic languages use singular with 21, 31, etc.

ngraham added inline comments.Mar 18 2019, 10:52 PM
applets/comic/package/contents/ui/configAdvanced.qml
55

Yep, even if the quantity will always be more than one, it should always use an i18np(). for just that reason. :)

applets/comic/package/contents/ui/configGeneral.qml
97

You could shorten this very long string by writing it as "Check for new plugins every:"

111

"comic strips" -> "comics"

120

"Mins" -> "Minutes" (etc)

filipf updated this revision to Diff 54453.Mar 20 2019, 8:41 PM
filipf marked an inline comment as done.

Change strings according to Nate's suggestions

filipf marked 3 inline comments as done.Mar 20 2019, 8:41 PM
abetts added a subscriber: abetts.Mar 20 2019, 8:44 PM

The title labels appear to be very far from the content. Is that just the size on the images or does it actually look very far when the config screen is full size?

The title labels appear to be very far from the content. Is that just the size on the images or does it actually look very far when the config screen is full size?

You mean "General", "Appearance" etc. right? Yeah that happens when windows are stretched horizontally, but the window wouldn't be much narrower IRL tbh. Kirigami's FormLayout is center aligned and the category's title always remains left aligned. They're pretty far apart in full-screen:

You mean "General", "Appearance" etc. right? Yeah that happens when windows are stretched horizontally, but the window wouldn't be much narrower IRL tbh. Kirigami's FormLayout is center aligned and the category's title always remains left aligned. They're pretty far apart in full-screen:

Awesome! Thank you, just wanted to be sure. +1

ngraham accepted this revision.Mar 20 2019, 10:05 PM
This revision is now accepted and ready to land.Mar 20 2019, 10:05 PM
This revision was automatically updated to reflect the committed changes.