D19802: [comic] Modernize configuration windows

Nathaniel Graham noreply at phabricator.kde.org
Sat Mar 16 10:37:32 GMT 2019


ngraham requested changes to this revision.
ngraham added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> configAdvanced.qml:55
> +        Controls.Label {
> +            text: maxComicLimit.value !==1 ? i18nc("@item:valuesuffix spacing to number + unit", " strips per comic") : i18nc("@item:valuesuffix spacing to number + unit", " strip per comic")
>          }

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")

> configAppearance.qml:55
> +        id: showComicTitle
> +        Kirigami.FormData.label: i18n("Show comic:")
> +        text: i18nc("@option:check", "Title")

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

> configGeneral.qml:110
> +        Controls.Label {
> +            text: providerUpdateInterval.value !==1 ? ("@item:valuesuffix spacing to number + unit", " days") : ("@item:valuesuffix spacing to number + unit", " day")
> +        }

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)

> configGeneral.qml:127
> +        Controls.Label {
> +            text: checkNewComicStripsInterval.value !==1 ? ("@item:valuesuffix spacing to number + unit (minutes)", " mins") : ("@item:valuesuffix spacing to number + unit (minutes)", " min")
> +        }

Ditto

> configGeneral.qml:88
> +        id: middleClickCheckBox
> +        text: i18nc("@option:check", "Middle-click on the comic to show it in its original size")
> +        onCheckedChanged: root.configurationChanged();

This could be shortened:

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

> configGeneral.qml:59
> +        Kirigami.FormData.isSection: true
> +        Kirigami.FormData.label: i18nc("@title:group", "Comics")
> +    }

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

> configGeneral.qml:97
> +
> +    Layouts.RowLayout {
> +

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

REPOSITORY
  R114 Plasma Addons

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

To: filipf, rooty, ngraham, #vdg, #plasma
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190316/5a256a8b/attachment-0001.html>


More information about the Plasma-devel mailing list