Skip to content
This repository has been archived by the owner on Mar 8, 2019. It is now read-only.

Work around standaloneDayName being incorrect in pt_PT #87

Merged
merged 3 commits into from
Jun 10, 2018

Conversation

UniversalSuperBox
Copy link
Member

This is a workaround for ubports/ubuntu-touch#510. It fixes half of the issue (the DateTime indicator uses another component).

Since the pt_PT locale has incorrect short day names, we can use the pt locale instead in this instance.

@dobey suggested that we remove pt_PT from our list of locales and instead use pt. However, it seems that the use of the specific locale was to work around QLocale("pt").nativeLanguageName() is "português do Brasil" in Qt.

This is a workaround for
ubports/ubuntu-touch#510

Since the pt_PT locale has incorrect short day names, we can use the pt
locale instead in this instance.
@Flohack74
Copy link
Member

Can you also fix the CI issues here xD

Copy link
Contributor

@dpniel dpniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of nitpicks

property var day : {
if (Qt.locale().name == "pt_PT") {
// Workaround for https://github.com/ubports/ubuntu-touch/issues/510
Qt.locale("pt").standaloneDayName(( Qt.locale().firstDayOfWeek + index), Locale.ShortFormat);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: When defining property bindings in { } you should use the return statement . See the second example here

} else {
Qt.locale(Qt.locale().name).standaloneDayName(( Qt.locale().firstDayOfWeek + index), Locale.ShortFormat);
}
}
text: isYearView ? Qt.locale(Qt.locale().name).standaloneDayName(( Qt.locale().firstDayOfWeek + index), Locale.NarrowFormat) : day
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than keep repeating the calls to Qt.locale().blah in multiple places. It might be a bit cleaner to do something like

readonly property string localeName: Qt.locale().name === "pt_PT" ? "pt" : Qt.locale().name

text: Qt.locale(localeName).standaloneDayName(Qt.locale().firstDayOfWeek + index, isYearView ? Locale.NarrowFormat : Locale.ShortFormat);

What do you think?

@@ -338,7 +338,14 @@ Item{
id: weekDay
objectName: "weekDay" + index
width: parent.dayWidth
property var day : Qt.locale(Qt.locale().name).standaloneDayName(( Qt.locale().firstDayOfWeek + index), Locale.ShortFormat);
property var day : {
if (Qt.locale().name == "pt_PT") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Wrong indentation ;-)

@UniversalSuperBox
Copy link
Member Author

We're trying to fix the underlying problem rather than working around it at ubports/system-settings#56 (comment). Stay tuned, I suppose.

@UniversalSuperBox
Copy link
Member Author

Fixing the underlying problem did not, in fact, work. Turns out that Qt assumes you mean pt_BR when you say pt. So I've fixed this workaround, if you are okay with it then this is how we proceed.

Copy link
Contributor

@dpniel dpniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@UniversalSuperBox
Copy link
Member Author

There, much better. :P

@NeoTheThird NeoTheThird merged commit 846af9f into master Jun 10, 2018
@NeoTheThird NeoTheThird deleted the workaround-ptPT branch June 10, 2018 08:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants