[Digital Clock] Add ability to set a custom date format string
ClosedPublic

Authored by Zren on Jan 6 2019, 6:23 PM.

Details

Summary

Adds a new customDateFormat config key which is used when the dateFormat "StringEnum" is set to custom.
Shows a link to the Qt time formatting documentation above the text field.
Qt doc link and text field are hidden when not set to custom date format.

CCBUG: 340982



Should I add presets like Event Calendar? https://store.kde.org/p/998901/

I left out the large text blurb that you can use Rich Text formatting, but Digital Clock can now support using '<font color="#f00">'ddd'</font>' d to change the color of the date.

Test Plan

Easily test by modifying /usr/share/plasma/plasmoids/org.kde.plasma.digitalclock/ and running plasmoidviewer -a org.kde.plasma.digitalclock.

  • Confirmed documentation link and TextField are hidden when not set to Custom.
  • Confirmed ComboBox selected Custom after closing and reopening the config window.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Zren created this revision.Jan 6 2019, 6:23 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 6 2019, 6:23 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
Zren requested review of this revision.Jan 6 2019, 6:23 PM
Zren edited the summary of this revision. (Show Details)Jan 6 2019, 6:33 PM
rooty added a subscriber: rooty.Jan 8 2019, 3:22 AM

a thousand times yes... this is amazing

Zren added inline comments.Jan 8 2019, 4:33 AM
applets/digital-clock/package/contents/ui/configAppearance.qml
145

Does this also need a i18nc("custom date format", "Custom") translation context?

ndavis added a subscriber: ndavis.EditedJan 8 2019, 4:44 AM

Should I add presets like Event Calendar?

I'm leaning towards no, but a second link to text formatting documentation might be a good idea.

Zren added a comment.Jan 8 2019, 5:49 AM

I'm leaning towards no, but a second link to text formatting documentation might be a good idea.

Adding a second link is pretty easy, though I wrote this before I noticed your edit. A second link would look better + simpler to maintain though. Though without a RichText example, the user might not understand the need to escape using apostrophes without an example ('<b>'ddd'</b>' ).

I'm leaning towards no, but an example of the custom time formatting and another of Rich Text formatting would be a good idea.

Here's an example table, using an unconventional method of drawing a grid.

QtLayouts.GridLayout {
    id: exampleFormatsTable
    columns: 2

    property var model: [
        "yyyy-MM-dd",
        "MMM d",
        "'<b>'ddd'</b>'d",
        "'<font color=\"#f00\">'ddd'</font>'d",
    ]

    Repeater {
        model: exampleFormatsTable.model.length * 2

        Item {
            id: cell
            property int padding: units.smallSpacing
            implicitWidth: exampleLabel.implicitWidth + padding*2
            implicitHeight: exampleLabel.implicitHeight + padding*2
            QtLayouts.Layout.fillWidth: true
            readonly property int row: Math.floor(index / exampleFormatsTable.columns)
            readonly property int column: index % exampleFormatsTable.columns

            Rectangle {
                anchors.fill: parent
                color: exampleLabel.color
                opacity: 0.1
            }

            QtControls.Label {
                id: exampleLabel
                anchors.left: parent.left
                anchors.top: parent.top
                anchors.margins: cell.padding

                textFormat: cell.column === 0 ? Text.PlainText : Text.RichText
                text: {
                    var exampleFormat = exampleFormatsTable.model[cell.row]
                    if (cell.column === 0) {
                        return exampleFormat
                    } else {
                        return Qt.formatDate(new Date(), exampleFormat)
                    }
                }
            }
        }

    }
}
ndavis added a comment.Jan 8 2019, 6:33 AM
In D18019#388819, @Zren wrote:

I'm leaning towards no, but a second link to text formatting documentation might be a good idea.

Adding a second link is pretty easy, though I wrote this before I noticed your edit. A second link would look better + simpler to maintain though. Though without a RichText example, the user might not understand the need to escape using apostrophes without an example ('<b>'ddd'</b>' ).

I'm leaning towards no, but an example of the custom time formatting and another of Rich Text formatting would be a good idea.

Here's an example table, using an unconventional method of drawing a grid.

Shouldn't the documentation explain that apostrophes need to be used or is the fact that they need to be used unusual and specific to this clock widget?

mart added a subscriber: mart.Jan 8 2019, 9:37 AM

is this the right place for it? (a text field to enter the "magic" date format letters is a very geeky ui, and i would prefer to not go there) shouldn't be a generic localization/date format setting? (and with also a graphical ui to generate this)

ndavis added a comment.EditedJan 8 2019, 10:18 AM
In D18019#388876, @mart wrote:

is this the right place for it? (a text field to enter the "magic" date format letters is a very geeky ui, and i would prefer to not go there) shouldn't be a generic localization/date format setting? (and with also a graphical ui to generate this)

That already exists as other combo box options. You can currently pick "Long Date" (localized), "Short Date" (localized) or "ISO Date". This adds the ability to do anything you want as an extra option.

Zren added a comment.EditedJan 8 2019, 3:41 PM

Shouldn't the documentation explain that apostrophes need to be used or is the fact that they need to be used unusual and specific to this clock widget?

The time format docs mentions:
http://doc.qt.io/qt-5/qml-qtqml-qt.html#formatDateTime-method

All other input characters will be ignored. Any sequence of characters that are enclosed in single quotes will be treated as text and not be used as an expression.

Which you can combine with QML's RichText documentation:
http://doc.qt.io/qt-5/qml-qtquick-text.html#textFormat-prop

The time format docs do not provide an example of using single quotes, and you'd need a person with a programmers mindset to use the escape character with the rich text docs to get the examples above.

A user is more likely to try and use <font color="#ddd">ddd</font> d which would turn into <font color="#Tue">Tue</font> 8 and give up... Though now that I think of it, ddd is the only color that would need to be escaped. aaa, bbb, ccc, eee, fff are not date format variables so they are ignored. font has a t (time zone), but since it has fon in front of it, the date formatting ignores it. <strong> is similar. color= is ignored since it has no variables in it. <b> and <i> is also ignored.

We could probably get away with just linking to the rich text documentation. I've been over-complicating it in Event Calendar for a while now it seems. Should the link's label be i18n("Text Color Documentation") or i18n("Color Formatting Documentation") or i18n("Rich Text Documentation")?


In D18019#388876, @mart wrote:

is this the right place for it? (a text field to enter the "magic" date format letters is a very geeky ui, and i would prefer to not go there) shouldn't be a generic localization/date format setting? (and with also a graphical ui to generate this)

The user can already change the Date Formatting globally to a specific Locale's time format (right click widget > Set Time Format...), which is limiting.

The widget's Long Format uses a ridiculous width on the panel:

While the Short Format does not display the day of the week, includes the year (which isn't useful after January) and since Canada/US puts the Month first, annoying to read since the month is represented as a number as well.

Changing the global Time Format requires the user to restart plasma to apply and see the changes directly in the panel. Changing the global format also isn't desired since people tend to use an over simplified date format in the panel (eg: Tue 31 or Jan 31), but will still want legible timestamps everywhere else (eg: Dolphin).

ngraham added a subscriber: ngraham.Jan 8 2019, 5:37 PM

"Very geeky options" aren't generally a problem as long as they're not the default setting. In fact, they're one of the big things that KDE is known for so in principle I think this is great as long as it doesn't get in the way of people who are not very geeky.

FWIW even Apple has this feature and allows people who are very particular about their date display to customize it however they want:

It is very hidden though, which is appropriate.

However, to @mart's point, the custom format chooser is systemwide on macOS, not limited to their digital clock equivalent. Systemwide does indeed seem like the more appropriate place, but I'm not sure if that would be compatible with Qt's locale system though?

cfeck added a subscriber: cfeck.Jan 8 2019, 10:56 PM

Bug 340982 is about system-wide locale format changes. Qt currently does not have a way to customize its internal CLDR database.

This change is indeed only about the Plasma clock, somewhat a workaround for the bug.

Darn. Thanks Christoph.

If global changes are currently impossible due to lack of support in Qt, I think this is a reasonable approach for now, since it's the most prominent place where formatted times and dates are actually displayed in the UI.

Therefore +1

abetts added a subscriber: abetts.Jan 8 2019, 11:01 PM

Is there a way that the custom date setting can be in its own window pop up instead of building into the KCM and pushing down the control that's below this setting?

ndavis added a comment.Jan 9 2019, 2:47 AM

I think some users may want to have a separate date format for their clock than the rest of the system. A user might prefer to use ISO date format with only the last 2 digits of the year in their clock because it saves space, but they might prefer to use regular ISO date format everywhere else. They may also not want a system wide date format with hardcoded custom text colors.

Zren added a comment.Jan 10 2019, 8:11 AM

The "show date" / "show seconds" toggles only affect the current widget instance as well. It would be nice if plasma had a plasmoid.globalConfiguration.dateFormatStr API which all widget instances (on all panels/screens) were bound to, so the user only had to configure it once for all screens. Though a person might have a clock on a vertical panel on one screen, and a thin horizontal panel on the other that might require a different configuration. That's an edge case example though.


I played around with a prettier looking example table.


However I couldn't settle on anything. We can focus on a fancy TextField with embedded documentation later.

For now though, I'd like to know this feature with the 2 documentation links can be merged. Is it to late to get added to Plasma 5.15? The Beta is next week.

Zren updated this revision to Diff 49137.Jan 10 2019, 8:24 AM
Zren edited the summary of this revision. (Show Details)

Translate the "Time Format Documentation" link.
Add a link to QML's Text RichText docs under the "Color Format Documentation" label (translated).
Place the 2 links below the text field.

I think these changes ultimately come down to two questions:

1) Is it something we actually want to customise at a per-plasmoid level, or is it a hack round something else?

I quite like ndavis's reply.

user might prefer to use ISO date format with only the last 2 digits of the year in their clock because it saves space, but they might prefer to use regular ISO date format everywhere else.

"show seconds" toggles only affect the current widget instance as well.

2) Does it put "complex" options into the default UI?

This doesn't, as it's hidden behind the "custom" setting in the drop down.

So for those reasons, I'm inclined to say +1.
I don't think the answers to the same question apply to the "First Day of the Week" patch.


I don't think we need the colour formatting link. It's a niche within a niche. Removing it "solves" the escaping issue as it means the user is doing something undocumented in the first place.

In D18019#390363, @Zren wrote:

I played around with a prettier looking example table.


This would be fantastic, especially if clicking on one added it to the text field, and especially if they're all draggable!

Zren updated this revision to Diff 49332.Jan 12 2019, 2:46 PM
Zren edited the summary of this revision. (Show Details)
Zren edited the test plan for this revision. (Show Details)
  • Rename config key from dateFormatStr and config TextField dateFormatTextField to customDateFormat.
  • Remove the "Color Format Documentation" link.
Zren added a comment.Jan 12 2019, 3:35 PM

D18155

Looking at the alternative patch, I've renamed the config key variable to customDateFormat. Most of the differences stem from not using a "variant" data type for DigitalClock.qml's dateFormat variable. The string formatDate(datetime date, variant format) (qt doc link) accepts a variant type.

So for those reasons, I'm inclined to say +1.

Assuming there's no objection to this, I'll merge this Sunday night. The beta is Thurs, so I assume that's the cutoff date?

I don't think the answers to the same question apply to the "First Day of the Week" patch.

Fair enough. With that setting, the user sort of expects every date selection calendar popup (Qt/Gtk) to follow that setting. Where as with this setting, the user's expectation is that it should only change the panel's clock widget.

I don't think we need the colour formatting link. It's a niche within a niche. Removing it "solves" the escaping issue as it means the user is doing something undocumented in the first place.

Sounds good. Until there's an embedded list of examples (D18168) it could just overwhelm the user with 2 different syntaxes.


Final note. We're using Qt.formatDate(), which means the "time formats" hh, mm, ss, t (time zone), and zzz (millis) are not replaced with their value. Implementing that would require duplicating the "sizehelper timeMetrics" code for the date format though, which I can't guarantee a bugless patch this soon before the 5.15 Beta. If I finish D18168, then it might help suggest to the user that only the "date variables" are accepted.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 14 2019, 3:24 AM
This revision was automatically updated to reflect the committed changes.

Why was this committed?

To pad out the 5.15 feature list, maybe? 😜

Zren added a comment.Jan 14 2019, 9:34 PM

There was a few +1s, but I wasn't exactly sure who specifically "approves" this for a merge to plasma-workspace or digital-clock specifically. My bad, I shouldn't of left this feature to the last minute and felt rushed to get it in. I'll try to avoid the time before the betas when submitting in the future.