From 244e06ee43295f11921d6bb60171a48b0eda4703 Mon Sep 17 00:00:00 2001 From: Nate Graham Date: Mon, 2 Nov 2020 15:06:38 -0700 Subject: [PATCH 1/4] [applets/digital-clock] Overhaul how to change between timezones Right now we have a usability problem: the Digital Clock applet has two ways to let you change the timezone displayed in the applet itself, but has no easy link to the correct way to change the timezone systemwide. As a result, users change the timezone in the clock and wonder why times are wrong everywhere else throughout the OS. This commit removes the feature of changing the timezone in just the clock, and replaces it with an overhauled Time Zones page in the config dialog and a new button in the popup that links you to the Date & Time KCM where you can change the systemwide time zone in the correct way. BUG: 428096 FIXED-IN: 5.21 --- .../package/contents/config/main.xml | 8 +- .../package/contents/ui/CalendarView.qml | 36 ++-- .../package/contents/ui/DigitalClock.qml | 38 ---- .../package/contents/ui/configTimeZones.qml | 181 +++++++++++++----- .../digital-clock/plugin/timezonemodel.cpp | 35 +++- applets/digital-clock/plugin/timezonemodel.h | 7 +- 6 files changed, 188 insertions(+), 117 deletions(-) diff --git a/applets/digital-clock/package/contents/config/main.xml b/applets/digital-clock/package/contents/config/main.xml index 76ee7ce55b..55077e31d0 100644 --- a/applets/digital-clock/package/contents/config/main.xml +++ b/applets/digital-clock/package/contents/config/main.xml @@ -42,17 +42,13 @@ default - + Local - + Local - - - false - true diff --git a/applets/digital-clock/package/contents/ui/CalendarView.qml b/applets/digital-clock/package/contents/ui/CalendarView.qml index ba35753956..11f8f6f18a 100644 --- a/applets/digital-clock/package/contents/ui/CalendarView.qml +++ b/applets/digital-clock/package/contents/ui/CalendarView.qml @@ -17,6 +17,8 @@ */ import QtQuick 2.4 import QtQuick.Layouts 1.1 + +import org.kde.kquickcontrolsaddons 2.0 // For kcmshell import org.kde.plasma.core 2.0 as PlasmaCore import org.kde.plasma.calendar 2.0 as PlasmaCalendar import org.kde.plasma.components 3.0 as PlasmaComponents3 @@ -429,17 +431,31 @@ PlasmaComponents3.Page { // Clocks stuff // ------------ - // Header text - PlasmaExtras.Heading { + // Header text + button to change time & timezone + RowLayout { + Layout.fillWidth: true visible: worldClocks.visible - Layout.fillWidth: true + PlasmaExtras.Heading { + Layout.fillWidth: true - level: 2 + level: 2 - text: i18n("Time Zones") - maximumLineCount: 1 - elide: Text.ElideRight + text: i18n("Time Zones") + maximumLineCount: 1 + elide: Text.ElideRight + } + + PlasmaComponents3.ToolButton { + visible: KCMShell.authorize("clock.desktop").length > 0 + text: i18n("Switch...") + icon.name: "preferences-system-time" + onClicked: KCMShell.openSystemSettings("clock") + + PlasmaComponents3.ToolTip { + text: i18n("Switch to another timezone") + } + } } // Clocks view itself @@ -473,12 +489,6 @@ PlasmaComponents3.Page { width: clocksList.width height: units.gridUnit + units.smallSpacing - MouseArea { - anchors.fill: parent - cursorShape: Qt.PointingHandCursor - onClicked: plasmoid.configuration.lastSelectedTimezone = modelData - } - RowLayout { anchors.fill: parent diff --git a/applets/digital-clock/package/contents/ui/DigitalClock.qml b/applets/digital-clock/package/contents/ui/DigitalClock.qml index a28bd017d3..1ff90e0a0b 100644 --- a/applets/digital-clock/package/contents/ui/DigitalClock.qml +++ b/applets/digital-clock/package/contents/ui/DigitalClock.qml @@ -411,46 +411,8 @@ Item { ] MouseArea { - id: mouseArea - - property int wheelDelta: 0 - anchors.fill: parent - onClicked: plasmoid.expanded = !plasmoid.expanded - onWheel: { - if (!plasmoid.configuration.wheelChangesTimezone) { - return; - } - - var delta = wheel.angleDelta.y || wheel.angleDelta.x - var newIndex = main.tzIndex; - wheelDelta += delta; - // magic number 120 for common "one click" - // See: https://doc.qt.io/qt-5/qml-qtquick-wheelevent.html#angleDelta-prop - while (wheelDelta >= 120) { - wheelDelta -= 120; - newIndex--; - } - while (wheelDelta <= -120) { - wheelDelta += 120; - newIndex++; - } - - if (newIndex >= plasmoid.configuration.selectedTimeZones.length) { - newIndex = 0; - } else if (newIndex < 0) { - newIndex = plasmoid.configuration.selectedTimeZones.length - 1; - } - - if (newIndex !== main.tzIndex) { - plasmoid.configuration.lastSelectedTimezone = plasmoid.configuration.selectedTimeZones[newIndex]; - main.tzIndex = newIndex; - - dataSource.dataChanged(); - setupLabels(); - } - } } /* diff --git a/applets/digital-clock/package/contents/ui/configTimeZones.qml b/applets/digital-clock/package/contents/ui/configTimeZones.qml index f3cd8bdce5..cae5c32311 100644 --- a/applets/digital-clock/package/contents/ui/configTimeZones.qml +++ b/applets/digital-clock/package/contents/ui/configTimeZones.qml @@ -23,84 +23,167 @@ import QtQuick.Controls 2.8 as QQC2 import QtQuick.Layouts 1.0 import QtQuick.Dialogs 1.1 +import org.kde.kquickcontrolsaddons 2.0 // For kcmshell import org.kde.plasma.private.digitalclock 1.0 import org.kde.plasma.core 2.0 as PlasmaCore -import org.kde.kirigami 2.8 as Kirigami +import org.kde.kirigami 2.14 as Kirigami ColumnLayout { id: timeZonesPage property alias cfg_selectedTimeZones: timeZones.selectedTimeZones - property alias cfg_wheelChangesTimezone: enableWheelCheckBox.checked TimeZoneModel { id: timeZones + } - onSelectedTimeZonesChanged: { - if (selectedTimeZones.length === 0) { - messageWidget.visible = true; + RowLayout { + Layout.rightMargin: Kirigami.Units.largeSpacing * 2 - timeZones.selectLocalTimeZone(); - } + Kirigami.Heading { + Layout.fillWidth: true + level: 2 + text: xi18nc("@info", "System Time Zone: %1", timeZones.systemTimeZone()) + elide: Text.ElideRight + } + + QQC2.Button { + visible: KCMShell.authorize("clock.desktop").length > 0 + text: i18n("Switch Time Zone...") + icon.name: "preferences-system-time" + onClicked: KCMShell.openSystemSettings("clock") } } - Kirigami.InlineMessage { - id: messageWidget - Layout.fillWidth: true - Layout.margins: Kirigami.Units.smallSpacing - type: Kirigami.MessageType.Warning - text: i18n("At least one time zone needs to be enabled. 'Local' was enabled automatically.") - showCloseButton: true + Item { + implicitHeight: Kirigami.Units.largeSpacing + } + + Kirigami.Heading { + level: 2 + text: i18n("Other Time Zones") + Layout.bottomMargin: Kirigami.Units.largeSpacing } - Kirigami.SearchField { - id: filter + QQC2.ScrollView { Layout.fillWidth: true + Layout.leftMargin: Kirigami.Units.largeSpacing * 2 + Layout.rightMargin: Kirigami.Units.largeSpacing * 2 + Layout.minimumHeight: Kirigami.Units.gridUnit * 8 + Layout.maximumHeight: Kirigami.Units.gridUnit * 17 + Component.onCompleted: background.visible = true // enable border + + ListView { + id: configuredTimezoneList + clip: true // Avoid visual glitches + focus: true // keyboard navigation + activeFocusOnTab: true // keyboard navigation + + model: TimeZoneFilterProxy { + sourceModel: timeZones + onlyShowChecked: true + } + + delegate: Kirigami.SwipeListItem { + width: configuredTimezoneList.width + // Don't need a highlight effect since the list item does + // nothing when clicked + activeBackgroundColor: "transparent" + contentItem: QQC2.Label { + text: model.city + } + actions: [ + Kirigami.Action { + tooltip: i18n("Remove this timezone") + iconName: "edit-delete" + onTriggered: model.checked = false; + } + ] + } + + Kirigami.PlaceholderMessage { + anchors.centerIn: parent + width: parent.width - (Kirigami.Units.LargeSpacing * 4) + visible: configuredTimezoneList.count == 0 + text: i18n("No other time zones configured") + helpfulAction: Kirigami.Action { + iconName: "list-add" + text: "Add Time Zones..." + onTriggered: timezoneSheet.open() + } + } + } + } + + QQC2.Button { + Layout.leftMargin: Kirigami.Units.largeSpacing * 2 + Layout.alignment: Qt.AlignLeft + visible: configuredTimezoneList.count > 0 + text: i18n("Add More Time Zones...") + icon.name: "list-add" + onClicked: timezoneSheet.open() } Item { - Layout.fillWidth: true + // Tighten up the layout Layout.fillHeight: true + } - QQC2.ScrollView { - anchors.fill: parent - clip: true - Component.onCompleted: background.visible = true // enable border + Kirigami.OverlaySheet { + id: timezoneSheet - ListView { - id: listView - focus: true // keyboard navigation - activeFocusOnTab: true // keyboard navigation + onSheetOpenChanged: { + filter.text = ""; + } - model: TimeZoneFilterProxy { - sourceModel: timeZones - filterString: filter.text - } + // Need to manually set the parent when using this in a Plasma config dialog + parent: timeZonesPage.parent - delegate: QQC2.CheckDelegate { - id: checkbox - focus: true // keyboard navigation - width: listView.width - text: !city || city.indexOf("UTC") === 0 ? comment : comment ? i18n("%1, %2 (%3)", city, region, comment) : i18n("%1, %2", city, region) - checked: model.checked - onToggled: { - model.checked = checkbox.checked - listView.currentIndex = index // highlight - listView.forceActiveFocus() // keyboard navigation - } - highlighted: ListView.isCurrentItem - } + // It interferes with the search field in the header + showCloseButton: false + + header: ColumnLayout { + Layout.preferredWidth: Kirigami.Units.gridUnit * 25 + + Kirigami.Heading { + text: i18n("Add More Timezones") + } + Kirigami.SearchField { + id: filter + Layout.fillWidth: true } } - } - RowLayout { - Layout.fillWidth: true - QQC2.CheckBox { - id: enableWheelCheckBox - text: i18n("Switch time zone with mouse wheel") + footer: QQC2.DialogButtonBox { + standardButtons: QQC2.DialogButtonBox.Ok + onAccepted: timezoneSheet.close() } - } + + ListView { + id: listView + focus: true // keyboard navigation + activeFocusOnTab: true // keyboard navigation + implicitWidth: Kirigami.Units.gridUnit * 25 + + model: TimeZoneFilterProxy { + sourceModel: timeZones + filterString: filter.text + } + + delegate: QQC2.CheckDelegate { + id: checkbox + width: listView.width + focus: true // keyboard navigation + text: !city || city.indexOf("UTC") === 0 ? comment : comment ? i18n("%1, %2 (%3)", city, region, comment) : i18n("%1, %2", city, region) + checked: model.checked + onToggled: { + model.checked = checkbox.checked + listView.currentIndex = index // highlight + listView.forceActiveFocus() // keyboard navigation + } + highlighted: ListView.isCurrentItem + } + } + } } diff --git a/applets/digital-clock/plugin/timezonemodel.cpp b/applets/digital-clock/plugin/timezonemodel.cpp index e7ee7ce0be..f6feaaa599 100644 --- a/applets/digital-clock/plugin/timezonemodel.cpp +++ b/applets/digital-clock/plugin/timezonemodel.cpp @@ -33,10 +33,21 @@ TimeZoneFilterProxy::TimeZoneFilterProxy(QObject *parent) bool TimeZoneFilterProxy::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const { - if (!sourceModel() || m_filterString.isEmpty()) { + if (!sourceModel()) { return true; } + // Always exclude the local timezone; we present this data elsewhere + const bool isLocalTimezone = sourceModel()->index(source_row, 0, source_parent).data(TimeZoneModel::TimeZoneIdRole).toString() == QStringLiteral("Local"); + if (isLocalTimezone) { + return false; + } + + const bool checked = sourceModel()->index(source_row, 0, source_parent).data(TimeZoneModel::CheckedRole).toBool(); + if (m_onlyShowChecked && !checked) { + return false; + } + const QString city = sourceModel()->index(source_row, 0, source_parent).data(TimeZoneModel::CityRole).toString(); const QString region = sourceModel()->index(source_row, 0, source_parent).data(TimeZoneModel::RegionRole).toString(); const QString comment = sourceModel()->index(source_row, 0, source_parent).data(TimeZoneModel::CommentRole).toString(); @@ -49,6 +60,7 @@ bool TimeZoneFilterProxy::filterAcceptsRow(int source_row, const QModelIndex &so return false; } + void TimeZoneFilterProxy::setFilterString(const QString &filterString) { m_filterString = filterString; @@ -57,6 +69,15 @@ void TimeZoneFilterProxy::setFilterString(const QString &filterString) invalidateFilter(); } +void TimeZoneFilterProxy::setOnlyShowChecked(const bool show) +{ + if (m_onlyShowChecked == show) { + return; + } + m_onlyShowChecked = show; + emit onlyShowCheckedChanged(); +} + //============================================================================= TimeZoneModel::TimeZoneModel(QObject *parent) @@ -201,15 +222,11 @@ void TimeZoneModel::setSelectedTimeZones(const QStringList &selectedTimeZones) sortTimeZones(); } -void TimeZoneModel::selectLocalTimeZone() +QString TimeZoneModel::systemTimeZone() { - m_data[0].checked = true; - - QModelIndex index = createIndex(0, 0); - emit dataChanged(index, index); - - m_selectedTimeZones << m_data[0].id; - emit selectedTimeZonesChanged(); + const QTimeZone localZone = QTimeZone(QTimeZone::systemTimeZoneId()); + const QStringList data = QString::fromUtf8(localZone.id()).split(QLatin1Char('/')); + return m_timezonesI18n->i18nCity(data.last()); } QHash TimeZoneModel::roleNames() const diff --git a/applets/digital-clock/plugin/timezonemodel.h b/applets/digital-clock/plugin/timezonemodel.h index fed9b1157d..3901b4d4c0 100644 --- a/applets/digital-clock/plugin/timezonemodel.h +++ b/applets/digital-clock/plugin/timezonemodel.h @@ -32,18 +32,22 @@ class TimeZoneFilterProxy : public QSortFilterProxyModel { Q_OBJECT Q_PROPERTY(QString filterString WRITE setFilterString MEMBER m_filterString NOTIFY filterStringChanged) + Q_PROPERTY(bool onlyShowChecked WRITE setOnlyShowChecked MEMBER m_onlyShowChecked NOTIFY onlyShowCheckedChanged) public: explicit TimeZoneFilterProxy(QObject *parent = nullptr); bool filterAcceptsRow(int source_row, const QModelIndex &source_parent) const override; void setFilterString(const QString &filterString); + void setOnlyShowChecked(const bool show); Q_SIGNALS: void filterStringChanged(); + void onlyShowCheckedChanged(); private: QString m_filterString; + bool m_onlyShowChecked; QStringMatcher m_stringMatcher; }; @@ -72,8 +76,7 @@ public: void update(); void setSelectedTimeZones(const QStringList &selectedTimeZones); - - Q_INVOKABLE void selectLocalTimeZone(); + Q_INVOKABLE QString systemTimeZone(); Q_SIGNALS: void selectedTimeZonesChanged(); -- GitLab From 41414dba5967965b2d3e957b4eb51ba2b6af0609 Mon Sep 17 00:00:00 2001 From: Nate Graham Date: Sat, 7 Nov 2020 15:07:35 -0700 Subject: [PATCH 2/4] Better UI with a single list and different sections --- .../package/contents/ui/configTimeZones.qml | 148 ++++++++++++------ applets/digital-clock/plugin/timezonedata.h | 1 + .../digital-clock/plugin/timezonemodel.cpp | 30 ++-- applets/digital-clock/plugin/timezonemodel.h | 6 +- 4 files changed, 119 insertions(+), 66 deletions(-) diff --git a/applets/digital-clock/package/contents/ui/configTimeZones.qml b/applets/digital-clock/package/contents/ui/configTimeZones.qml index cae5c32311..e0df3e91c2 100644 --- a/applets/digital-clock/package/contents/ui/configTimeZones.qml +++ b/applets/digital-clock/package/contents/ui/configTimeZones.qml @@ -35,42 +35,31 @@ ColumnLayout { TimeZoneModel { id: timeZones - } - - RowLayout { - Layout.rightMargin: Kirigami.Units.largeSpacing * 2 - - Kirigami.Heading { - Layout.fillWidth: true - level: 2 - text: xi18nc("@info", "System Time Zone: %1", timeZones.systemTimeZone()) - elide: Text.ElideRight - } - - QQC2.Button { - visible: KCMShell.authorize("clock.desktop").length > 0 - text: i18n("Switch Time Zone...") - icon.name: "preferences-system-time" - onClicked: KCMShell.openSystemSettings("clock") + onSelectedTimeZonesChanged: { + if (selectedTimeZones.length === 0) { + // Don't let the user remove all time zones + noTimeZonesMessage.visible = true; + timeZones.selectLocalTimeZone(); + } else if (cfg_selectedTimeZones.length > 1) { + // Don't let the user have no local time zone when there's more + // than one non-local time zone + if (!cfg_selectedTimeZones.includes("Local")) { + localTimeZoneRequiredRightNowMessage.visible = true; + timeZones.selectLocalTimeZone(); + } + // Ensure that the applet shows showing the local time if there + // are multiple timezones configured + if (plasmoid.configuration.lastSelectedTimezone != "Local") { + plasmoid.configuration.lastSelectedTimezone = "Local"; + } + } } } - Item { - implicitHeight: Kirigami.Units.largeSpacing - } - - Kirigami.Heading { - level: 2 - text: i18n("Other Time Zones") - Layout.bottomMargin: Kirigami.Units.largeSpacing - } - QQC2.ScrollView { Layout.fillWidth: true - Layout.leftMargin: Kirigami.Units.largeSpacing * 2 - Layout.rightMargin: Kirigami.Units.largeSpacing * 2 - Layout.minimumHeight: Kirigami.Units.gridUnit * 8 - Layout.maximumHeight: Kirigami.Units.gridUnit * 17 + Layout.minimumHeight: Kirigami.Units.gridUnit * 19 + Layout.maximumHeight: Kirigami.Units.gridUnit * 19 Component.onCompleted: background.visible = true // enable border ListView { @@ -89,11 +78,32 @@ ColumnLayout { // Don't need a highlight effect since the list item does // nothing when clicked activeBackgroundColor: "transparent" - contentItem: QQC2.Label { - text: model.city + contentItem: Kirigami.BasicListItem { + // Otherwise there are unnecessary margins + anchors.top: parent.top + anchors.bottom: parent.bottom + label: model.city + subtitle: model.comment.length > 0 ? model.comment : "" + activeBackgroundColor: "transparent" + textColor: Kirigami.Theme.textColor } actions: [ Kirigami.Action { + visible: { + if (configuredTimezoneList.count === 1) { + // Never makes sense to delete the only time zone + return false; + } else if (configuredTimezoneList.count === 2) { + // When there are only two tone zones, let the + // user delete either one, so they can configure + // the applet to display a non-local time zone + return true; + } else { + // Don't let the user delete the local time zone + // when there's more than one non-local time zone + return !model.isLocalTimeZone; + } + } tooltip: i18n("Remove this timezone") iconName: "edit-delete" onTriggered: model.checked = false; @@ -101,27 +111,51 @@ ColumnLayout { ] } - Kirigami.PlaceholderMessage { - anchors.centerIn: parent - width: parent.width - (Kirigami.Units.LargeSpacing * 4) - visible: configuredTimezoneList.count == 0 - text: i18n("No other time zones configured") - helpfulAction: Kirigami.Action { - iconName: "list-add" - text: "Add Time Zones..." - onTriggered: timezoneSheet.open() + section { + property: "isLocalTimeZone" + delegate: Kirigami.ListSectionHeader { + label: section == "true" || !cfg_selectedTimeZones.includes("Local") ? i18n("Time zone used for clock display") : i18n("Additional time zones in pop-up") } } } } - QQC2.Button { - Layout.leftMargin: Kirigami.Units.largeSpacing * 2 - Layout.alignment: Qt.AlignLeft - visible: configuredTimezoneList.count > 0 - text: i18n("Add More Time Zones...") - icon.name: "list-add" - onClicked: timezoneSheet.open() + RowLayout { + Layout.fillWidth: true + + QQC2.Button { + text: i18n("Add Time Zones...") + icon.name: "list-add" + onClicked: timezoneSheet.open() + } + + Item { + Layout.fillWidth: true + } + + QQC2.Button { + visible: KCMShell.authorize("clock.desktop").length > 0 + text: i18n("Switch Local Time Zone...") + icon.name: "preferences-system-time" + onClicked: KCMShell.openSystemSettings("clock") + } + } + + QQC2.Label { + id: nonLocalTimeZoneMessage + visible: cfg_selectedTimeZones.includes("Local") + Layout.fillWidth: true + Layout.margins: Kirigami.Units.largeSpacing * 2 + text: i18n("To have the clock reflect a different time zone, put a single other time zone in the above list, and then remove the local timezone.") + wrapMode: Text.Wrap + } + + QQC2.Label { + visible: !nonLocalTimeZoneMessage.visible + Layout.fillWidth: true + Layout.margins: Kirigami.Units.largeSpacing * 2 + text: i18n("To have the clock reflect the local time zone, add the local time zone to the above list.") + wrapMode: Text.Wrap } Item { @@ -134,6 +168,7 @@ ColumnLayout { onSheetOpenChanged: { filter.text = ""; + noTimeZonesMessage.visible = false; } // Need to manually set the parent when using this in a Plasma config dialog @@ -152,6 +187,20 @@ ColumnLayout { id: filter Layout.fillWidth: true } + Kirigami.InlineMessage { + id: noTimeZonesMessage + Layout.fillWidth: true + type: Kirigami.MessageType.Warning + text: i18n("At least one time zone needs to be enabled. Your local timezone was enabled automatically.") + showCloseButton: true + } + Kirigami.InlineMessage { + id: localTimeZoneRequiredRightNowMessage + Layout.fillWidth: true + type: Kirigami.MessageType.Warning + text: i18n("The local timezone must be enabled when displaying more than one other time zone. Re-enabling the local time zone.") + showCloseButton: true + } } footer: QQC2.DialogButtonBox { @@ -159,7 +208,6 @@ ColumnLayout { onAccepted: timezoneSheet.close() } - ListView { id: listView focus: true // keyboard navigation diff --git a/applets/digital-clock/plugin/timezonedata.h b/applets/digital-clock/plugin/timezonedata.h index 4f609b73e8..08e41e4eb9 100644 --- a/applets/digital-clock/plugin/timezonedata.h +++ b/applets/digital-clock/plugin/timezonedata.h @@ -31,6 +31,7 @@ public: QString city; QString comment; bool checked; + bool isLocalTimeZone; int offsetFromUtc; }; diff --git a/applets/digital-clock/plugin/timezonemodel.cpp b/applets/digital-clock/plugin/timezonemodel.cpp index f6feaaa599..7a7fa660bf 100644 --- a/applets/digital-clock/plugin/timezonemodel.cpp +++ b/applets/digital-clock/plugin/timezonemodel.cpp @@ -33,16 +33,10 @@ TimeZoneFilterProxy::TimeZoneFilterProxy(QObject *parent) bool TimeZoneFilterProxy::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const { - if (!sourceModel()) { + if (!sourceModel() || (m_filterString.isEmpty() && !m_onlyShowChecked)) { return true; } - // Always exclude the local timezone; we present this data elsewhere - const bool isLocalTimezone = sourceModel()->index(source_row, 0, source_parent).data(TimeZoneModel::TimeZoneIdRole).toString() == QStringLiteral("Local"); - if (isLocalTimezone) { - return false; - } - const bool checked = sourceModel()->index(source_row, 0, source_parent).data(TimeZoneModel::CheckedRole).toBool(); if (m_onlyShowChecked && !checked) { return false; @@ -60,7 +54,6 @@ bool TimeZoneFilterProxy::filterAcceptsRow(int source_row, const QModelIndex &so return false; } - void TimeZoneFilterProxy::setFilterString(const QString &filterString) { m_filterString = filterString; @@ -113,6 +106,8 @@ QVariant TimeZoneModel::data(const QModelIndex &index, int role) const return currentData.comment; case CheckedRole: return currentData.checked; + case IsLocalTimeZoneRole: + return currentData.isLocalTimeZone; } } @@ -155,10 +150,11 @@ void TimeZoneModel::update() const QStringList data = QString::fromUtf8(localZone.id()).split(QLatin1Char('/')); TimeZoneData local; + local.isLocalTimeZone = true; local.id = QStringLiteral("Local"); local.region = i18nc("This means \"Local Timezone\"", "Local"); local.city = m_timezonesI18n->i18nCity(data.last()); - local.comment = i18n("Your system time zone"); + local.comment = i18n("System's local time zone"); local.checked = false; m_data.append(local); @@ -193,6 +189,7 @@ void TimeZoneModel::update() const QStringList cityCountryContinent = key.split(QLatin1Char('|')); TimeZoneData newData; + newData.isLocalTimeZone = false; newData.id = timeZone.id(); newData.region = timeZone.country() == QLocale::AnyCountry ? QString() : m_timezonesI18n->i18nContinents(cityCountryContinent.at(2)) + QLatin1Char('/') + m_timezonesI18n->i18nCountry(timeZone.country()); @@ -222,11 +219,15 @@ void TimeZoneModel::setSelectedTimeZones(const QStringList &selectedTimeZones) sortTimeZones(); } -QString TimeZoneModel::systemTimeZone() +void TimeZoneModel::selectLocalTimeZone() { - const QTimeZone localZone = QTimeZone(QTimeZone::systemTimeZoneId()); - const QStringList data = QString::fromUtf8(localZone.id()).split(QLatin1Char('/')); - return m_timezonesI18n->i18nCity(data.last()); + m_data[0].checked = true; + + QModelIndex index = createIndex(0, 0); + emit dataChanged(index, index); + + m_selectedTimeZones << m_data[0].id; + emit selectedTimeZonesChanged(); } QHash TimeZoneModel::roleNames() const @@ -236,7 +237,8 @@ QHash TimeZoneModel::roleNames() const {RegionRole, "region"}, {CityRole, "city"}, {CommentRole, "comment"}, - {CheckedRole, "checked"} + {CheckedRole, "checked"}, + {IsLocalTimeZoneRole, "isLocalTimeZone"} }); } diff --git a/applets/digital-clock/plugin/timezonemodel.h b/applets/digital-clock/plugin/timezonemodel.h index 3901b4d4c0..9af8c4f7ee 100644 --- a/applets/digital-clock/plugin/timezonemodel.h +++ b/applets/digital-clock/plugin/timezonemodel.h @@ -67,7 +67,8 @@ public: RegionRole, CityRole, CommentRole, - CheckedRole + CheckedRole, + IsLocalTimeZoneRole }; int rowCount(const QModelIndex &parent) const override; @@ -76,7 +77,8 @@ public: void update(); void setSelectedTimeZones(const QStringList &selectedTimeZones); - Q_INVOKABLE QString systemTimeZone(); + + Q_INVOKABLE void selectLocalTimeZone(); Q_SIGNALS: void selectedTimeZonesChanged(); -- GitLab From f87c82f6f643586c50a66dce6a381ba57db704ef Mon Sep 17 00:00:00 2001 From: Nate Graham Date: Sun, 8 Nov 2020 07:20:36 -0700 Subject: [PATCH 3/4] Refine the UI some more These separate commits are for my own sake so I can easily revert to a previous state in case we want to go back to an older version of the UI; I will squash the whole MR if and when it lands. --- .../package/contents/ui/configTimeZones.qml | 147 ++++++++---------- 1 file changed, 62 insertions(+), 85 deletions(-) diff --git a/applets/digital-clock/package/contents/ui/configTimeZones.qml b/applets/digital-clock/package/contents/ui/configTimeZones.qml index e0df3e91c2..21e6b2c2a4 100644 --- a/applets/digital-clock/package/contents/ui/configTimeZones.qml +++ b/applets/digital-clock/package/contents/ui/configTimeZones.qml @@ -34,32 +34,20 @@ ColumnLayout { property alias cfg_selectedTimeZones: timeZones.selectedTimeZones TimeZoneModel { + id: timeZones onSelectedTimeZonesChanged: { if (selectedTimeZones.length === 0) { // Don't let the user remove all time zones - noTimeZonesMessage.visible = true; + messageWidget.visible = true; timeZones.selectLocalTimeZone(); - } else if (cfg_selectedTimeZones.length > 1) { - // Don't let the user have no local time zone when there's more - // than one non-local time zone - if (!cfg_selectedTimeZones.includes("Local")) { - localTimeZoneRequiredRightNowMessage.visible = true; - timeZones.selectLocalTimeZone(); - } - // Ensure that the applet shows showing the local time if there - // are multiple timezones configured - if (plasmoid.configuration.lastSelectedTimezone != "Local") { - plasmoid.configuration.lastSelectedTimezone = "Local"; - } } } } QQC2.ScrollView { Layout.fillWidth: true - Layout.minimumHeight: Kirigami.Units.gridUnit * 19 - Layout.maximumHeight: Kirigami.Units.gridUnit * 19 + Layout.preferredHeight: Kirigami.Units.gridUnit * 19 Component.onCompleted: background.visible = true // enable border ListView { @@ -73,88 +61,82 @@ ColumnLayout { onlyShowChecked: true } - delegate: Kirigami.SwipeListItem { + // Using a hand-rolled delegate because Kirigami.BasicListItem doesn't + // support being given extra items to display on the end + delegate: Kirigami.AbstractListItem { width: configuredTimezoneList.width // Don't need a highlight effect since the list item does // nothing when clicked activeBackgroundColor: "transparent" - contentItem: Kirigami.BasicListItem { - // Otherwise there are unnecessary margins - anchors.top: parent.top - anchors.bottom: parent.bottom - label: model.city - subtitle: model.comment.length > 0 ? model.comment : "" - activeBackgroundColor: "transparent" - textColor: Kirigami.Theme.textColor - } - actions: [ - Kirigami.Action { - visible: { - if (configuredTimezoneList.count === 1) { - // Never makes sense to delete the only time zone - return false; - } else if (configuredTimezoneList.count === 2) { - // When there are only two tone zones, let the - // user delete either one, so they can configure - // the applet to display a non-local time zone - return true; - } else { - // Don't let the user delete the local time zone - // when there's more than one non-local time zone - return !model.isLocalTimeZone; - } + contentItem: RowLayout { + ColumnLayout { + Layout.minimumHeight: Kirigami.Units.iconSizes.large + Layout.fillWidth: true + Layout.alignment: Qt.AlignVCenter + QQC2.Label { + Layout.fillWidth: true + text: model.city + font.bold: plasmoid.configuration.lastSelectedTimezone === model.timeZoneId && configuredTimezoneList.count > 1 + elide: Text.ElideRight + } + QQC2.Label { + Layout.fillWidth: true + Layout.alignment: Qt.AlignTop + text: plasmoid.configuration.lastSelectedTimezone === model.timeZoneId && configuredTimezoneList.count > 1 ? "Clock is currently using this time zone" : "" + elide: Text.ElideRight + font: Kirigami.Theme.smallFont + opacity: 0.7 + visible: text.length > 0 + } + } + QQC2.Button { + visible: model.isLocalTimeZone && KCMShell.authorize("clock.desktop").length > 0 + text: i18n("Switch Local Time Zone...") + icon.name: "preferences-system-time" + onClicked: KCMShell.openSystemSettings("clock") + } + QQC2.Button { + visible: !model.isLocalTimeZone && configuredTimezoneList.count > 1 + icon.name: "edit-delete" + onClicked: model.checked = false; + QQC2.ToolTip { + text: i18n("Remove this time zone") } - tooltip: i18n("Remove this timezone") - iconName: "edit-delete" - onTriggered: model.checked = false; } - ] + QQC2.Button { + visible: configuredTimezoneList.count > 1 + icon.name: "object-select-symbolic" + checkable: true + checked: plasmoid.configuration.lastSelectedTimezone === model.timeZoneId + onToggled: plasmoid.configuration.lastSelectedTimezone = model.timeZoneId + QQC2.ToolTip { + text: i18n("Use this time zone for the clock") + } + } + } } section { property: "isLocalTimeZone" delegate: Kirigami.ListSectionHeader { - label: section == "true" || !cfg_selectedTimeZones.includes("Local") ? i18n("Time zone used for clock display") : i18n("Additional time zones in pop-up") + label: section == "true" ? i18n("System's Local Time Zone") : i18n("Additional Time Zones Shown in Pop-Up & Tooltip") } } } } - RowLayout { - Layout.fillWidth: true - - QQC2.Button { - text: i18n("Add Time Zones...") - icon.name: "list-add" - onClicked: timezoneSheet.open() - } - - Item { - Layout.fillWidth: true - } - - QQC2.Button { - visible: KCMShell.authorize("clock.desktop").length > 0 - text: i18n("Switch Local Time Zone...") - icon.name: "preferences-system-time" - onClicked: KCMShell.openSystemSettings("clock") - } - } - - QQC2.Label { - id: nonLocalTimeZoneMessage - visible: cfg_selectedTimeZones.includes("Local") - Layout.fillWidth: true - Layout.margins: Kirigami.Units.largeSpacing * 2 - text: i18n("To have the clock reflect a different time zone, put a single other time zone in the above list, and then remove the local timezone.") - wrapMode: Text.Wrap + QQC2.Button { + Layout.alignment: Qt.AlignLeft // Explicitly set so it gets reversed for LTR mode + text: i18n("Add Time Zones...") + icon.name: "list-add" + onClicked: timezoneSheet.open() } QQC2.Label { - visible: !nonLocalTimeZoneMessage.visible + visible: configuredTimezoneList.count > 1 Layout.fillWidth: true Layout.margins: Kirigami.Units.largeSpacing * 2 - text: i18n("To have the clock reflect the local time zone, add the local time zone to the above list.") + text: i18n("Note that using a different time zone for the clock does not change the systemwide local time zone. When you travel, switch the systemwide local time zone instead.") wrapMode: Text.Wrap } @@ -168,7 +150,7 @@ ColumnLayout { onSheetOpenChanged: { filter.text = ""; - noTimeZonesMessage.visible = false; + messageWidget.visible = false; } // Need to manually set the parent when using this in a Plasma config dialog @@ -181,26 +163,21 @@ ColumnLayout { Layout.preferredWidth: Kirigami.Units.gridUnit * 25 Kirigami.Heading { + Layout.fillWidth: true text: i18n("Add More Timezones") + wrapMode: Text.Wrap } Kirigami.SearchField { id: filter Layout.fillWidth: true } Kirigami.InlineMessage { - id: noTimeZonesMessage + id: messageWidget Layout.fillWidth: true type: Kirigami.MessageType.Warning text: i18n("At least one time zone needs to be enabled. Your local timezone was enabled automatically.") showCloseButton: true } - Kirigami.InlineMessage { - id: localTimeZoneRequiredRightNowMessage - Layout.fillWidth: true - type: Kirigami.MessageType.Warning - text: i18n("The local timezone must be enabled when displaying more than one other time zone. Re-enabling the local time zone.") - showCloseButton: true - } } footer: QQC2.DialogButtonBox { -- GitLab From 2b7ce43b8d905771c9dec652456bf8583bd11a81 Mon Sep 17 00:00:00 2001 From: Nate Graham Date: Sun, 8 Nov 2020 18:43:42 -0700 Subject: [PATCH 4/4] Use radio buttons instead of custom checkable pushbuttons --- .../package/contents/ui/configTimeZones.qml | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/applets/digital-clock/package/contents/ui/configTimeZones.qml b/applets/digital-clock/package/contents/ui/configTimeZones.qml index 21e6b2c2a4..1973714b65 100644 --- a/applets/digital-clock/package/contents/ui/configTimeZones.qml +++ b/applets/digital-clock/package/contents/ui/configTimeZones.qml @@ -69,6 +69,11 @@ ColumnLayout { // nothing when clicked activeBackgroundColor: "transparent" contentItem: RowLayout { + QQC2.RadioButton { + visible: configuredTimezoneList.count > 1 + checked: plasmoid.configuration.lastSelectedTimezone === model.timeZoneId + onToggled: plasmoid.configuration.lastSelectedTimezone = model.timeZoneId + } ColumnLayout { Layout.minimumHeight: Kirigami.Units.iconSizes.large Layout.fillWidth: true @@ -76,7 +81,6 @@ ColumnLayout { QQC2.Label { Layout.fillWidth: true text: model.city - font.bold: plasmoid.configuration.lastSelectedTimezone === model.timeZoneId && configuredTimezoneList.count > 1 elide: Text.ElideRight } QQC2.Label { @@ -103,24 +107,25 @@ ColumnLayout { text: i18n("Remove this time zone") } } - QQC2.Button { - visible: configuredTimezoneList.count > 1 - icon.name: "object-select-symbolic" - checkable: true - checked: plasmoid.configuration.lastSelectedTimezone === model.timeZoneId - onToggled: plasmoid.configuration.lastSelectedTimezone = model.timeZoneId - QQC2.ToolTip { - text: i18n("Use this time zone for the clock") - } - } } } section { property: "isLocalTimeZone" delegate: Kirigami.ListSectionHeader { - label: section == "true" ? i18n("System's Local Time Zone") : i18n("Additional Time Zones Shown in Pop-Up & Tooltip") + label: section == "true" ? i18n("System's Local Time Zone") : i18n("Additional Time Zones") + } + } + + Kirigami.PlaceholderMessage { + visible: configuredTimezoneList.count === 1 + anchors { + horizontalCenter: parent.horizontalCenter + bottom: parent.bottom + bottomMargin: Kirigami.Units.gridUnit * 7 } + width: parent.width - (Kirigami.Units.largeSpacing * 12) + text: i18n("Add more time zones to display all of them in the applet's pop-up, or use one of them for the clock itself") } } } @@ -136,7 +141,7 @@ ColumnLayout { visible: configuredTimezoneList.count > 1 Layout.fillWidth: true Layout.margins: Kirigami.Units.largeSpacing * 2 - text: i18n("Note that using a different time zone for the clock does not change the systemwide local time zone. When you travel, switch the systemwide local time zone instead.") + text: i18n("Note that using a different time zone for the clock does not change the systemwide local time zone. When you travel, switch the local time zone instead.") wrapMode: Text.Wrap } -- GitLab