Merge lp://qastaging/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting into lp://qastaging/~ubuntu-shorts-dev/ubuntu-rssreader-app/trunk

Proposed by Dariusz Gadomski
Status: Merged
Approved by: Joey Chan
Approved revision: 21
Merged at revision: 19
Proposed branch: lp://qastaging/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting
Merge into: lp://qastaging/~ubuntu-shorts-dev/ubuntu-rssreader-app/trunk
Diff against target: 467 lines (+109/-40)
10 files modified
ArticleFullImg.qml (+4/-3)
ArticleOneImgA.qml (+4/-3)
ArticleOneImgB.qml (+4/-3)
ArticleTextA.qml (+4/-3)
ArticleTextB.qml (+4/-3)
ArticleTwoImgA.qml (+4/-3)
ListColumnDelegate.qml (+2/-2)
ListColumnView.qml (+1/-1)
addDelegate.js (+19/-19)
dateutils.js (+63/-0)
To merge this branch: bzr merge lp://qastaging/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Ubuntu Shorts Developers Pending
Review via email: mp+176051@code.qastaging.launchpad.net

Commit message

Added formatting for the pubDate labels.

Description of the change

Added time formatting to Shorts.

If the article has been published less that 30 days ago
the time is expressed as number of either seconds, minutes,
hours or days ago. If the pubDate is over 30 days the time
is displayed as date using format: "MMMM Do".

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) :
review: Approve (continuous-integration)
Revision history for this message
Joey Chan (qqworini) wrote :

Thanks for the time format tool :)

Revision history for this message
Roman Shchekin (mrqtros) wrote :

Hi, thanks :)
But I think that this tool will be totally redesigned later because of a
lot of cultures...
Just imagine problems with (for example) Arabic culture. Even direction of
text changes :)

Best Regards,
Roman.

2013/7/21 Joey Chan <email address hidden>

> Thanks for the time format tool :)
> --
>
> https://code.launchpad.net/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting/+merge/176051
> Your team Ubuntu RSS Feed Reader Developers is requested to review the
> proposed merge of
> lp:~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting into
> lp:ubuntu-rssreader-app.
>

Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

Hi,

Thanks for you opinion. Could you please explain (or provide a link to any docs) how to improve it? I have only experience with pure (no gettext) Qt/QML apps for MeeGo utilizing qsTr in both: *.qml and *.js files. I did not have any clue how to implement i18n in *.js, that's why I pass i18n object there from QML.

In case of MeeGo using an array of strings along with qsTr (like the in line 463 in the diff below) worked fine for all languages (all the culture-specific stuff was done by translators: e.g. inserting Right-To-Left characters etc.).

Do you know any examples of i18n usage in *.js files?

Thanks,
Dariusz

Revision history for this message
David Planella (dpm) wrote :

Al 21/07/13 21:17, En/na Dariusz Gadomski ha escrit:
> Hi,
>
> Thanks for you opinion. Could you please explain (or provide a link to any docs) how to improve it? I have only experience with pure (no gettext) Qt/QML apps for MeeGo utilizing qsTr in both: *.qml and *.js files. I did not have any clue how to implement i18n in *.js, that's why I pass i18n object there from QML.
>
> In case of MeeGo using an array of strings along with qsTr (like the in line 463 in the diff below) worked fine for all languages (all the culture-specific stuff was done by translators: e.g. inserting Right-To-Left characters etc.).
>
> Do you know any examples of i18n usage in *.js files?
>
> Thanks,
> Dariusz
>

Hi Dariusz,

Yeah, you can use the regular i18n.tr() functions [1] from the Ubuntu
SDK, either in QML or JS files.

You can have a look at clock, weather and other core apps for the usage
of the i18n.tr() function. Also clock and calendar have good examples of
locale-aware times and numbers.

Cheers,
David.

[1]
http://developer.ubuntu.com/api/devel/ubuntu-13.10/qml/ui-toolkit/qml-ubuntu-components0-i18n.html

Revision history for this message
David Planella (dpm) wrote :

Al 21/07/13 21:17, En/na Dariusz Gadomski ha escrit:
> Hi,
>
> Thanks for you opinion. Could you please explain (or provide a link to any docs) how to improve it? I have only experience with pure (no gettext) Qt/QML apps for MeeGo utilizing qsTr in both: *.qml and *.js files. I did not have any clue how to implement i18n in *.js, that's why I pass i18n object there from QML.
>
> In case of MeeGo using an array of strings along with qsTr (like the in line 463 in the diff below) worked fine for all languages (all the culture-specific stuff was done by translators: e.g. inserting Right-To-Left characters etc.).
>
> Do you know any examples of i18n usage in *.js files?
>
> Thanks,
> Dariusz
>

Also, I've just noticed that you're reimplementing a relative time
calculation that is already done in the Calculator app.

It might be worth looking at using just one way of calculating this.
Looking at both at a glance, yours looks simpler and cleaner, but the
calculator one handles translations and plurals for all languages.

Do you think you could have a look at unifying the two functions?

Cheers,
David.

Revision history for this message
Roman Shchekin (mrqtros) wrote :

Yep, I think we must use "strong" realization rather than a liitle bit "naive" =)

P.S. David, could your please have a look at my merge proposal? =)
I can't provide you with url, but my branch is named stable-trunk-prepare2 as you may remember.
Testing fails, I don't know why :(

22.07.13 10:23 David Planella написал(а):

Al 21/07/13 21:17, En/na Dariusz Gadomski ha escrit:
> Hi,
>
> Thanks for you opinion. Could you please explain (or provide a link to any docs) how to improve it? I have only experience with pure (no gettext) Qt/QML apps for MeeGo utilizing qsTr in both: *.qml and *.js files. I did not have any clue how to implement i18n in *.js, that's why I pass i18n object there from QML.
>
> In case of MeeGo using an array of strings along with qsTr (like the in line 463 in the diff below) worked fine for all languages (all the culture-specific stuff was done by translators: e.g. inserting Right-To-Left characters etc.).
>
> Do you know any examples of i18n usage in *.js files?
>
> Thanks,
> Dariusz
>

Also, I've just noticed that you're reimplementing a relative time
calculation that is already done in the Calculator app.

It might be worth looking at using just one way of calculating this.
Looking at both at a glance, yours looks simpler and cleaner, but the
calculator one handles translations and plurals for all languages.

Do you think you could have a look at unifying the two functions?

Cheers,
David.

--
https://code.launchpad.net/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting/+merge/176051

Your team Ubuntu RSS Feed Reader Developers is requested to review the proposed merge of lp:~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting into lp:ubuntu-rssreader-app.

Revision history for this message
David Planella (dpm) wrote :

Al 22/07/13 08:34, En/na Roman Shchekin ha escrit:
> Yep, I think we must use "strong" realization rather than a liitle bit "naive" =)
>
> P.S. David, could your please have a look at my merge proposal? =)
> I can't provide you with url, but my branch is named stable-trunk-prepare2 as you may remember.
> Testing fails, I don't know why :(

Hi Roman,

I already commented on your other branch this morning.

Cheers,
David.

>
> 22.07.13 10:23 David Planella написал(а):
>
> Al 21/07/13 21:17, En/na Dariusz Gadomski ha escrit:
>> Hi,
>>
>> Thanks for you opinion. Could you please explain (or provide a link to any docs) how to improve it? I have only experience with pure (no gettext) Qt/QML apps for MeeGo utilizing qsTr in both: *.qml and *.js files. I did not have any clue how to implement i18n in *.js, that's why I pass i18n object there from QML.
>>
>> In case of MeeGo using an array of strings along with qsTr (like the in line 463 in the diff below) worked fine for all languages (all the culture-specific stuff was done by translators: e.g. inserting Right-To-Left characters etc.).
>>
>> Do you know any examples of i18n usage in *.js files?
>>
>> Thanks,
>> Dariusz
>>
>
> Also, I've just noticed that you're reimplementing a relative time
> calculation that is already done in the Calculator app.
>
>
> It might be worth looking at using just one way of calculating this.
> Looking at both at a glance, yours looks simpler and cleaner, but the
> calculator one handles translations and plurals for all languages.
>
>
> Do you think you could have a look at unifying the two functions?
>
>
> Cheers,
> David.
>

Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

I would love to improve my proposal, but I still miss some info.

In the calculator app I can see the internationalization implemented in Storage.qml file as part of the Storage component. I wanted to have it isolated in a *.js library in rssreader. I was not able to find out how to use i18n object properly in *.js files, that is why I pass 'i18n' object form QML to JS and use it there.

David: do you think it will not work in terms of internationalization/plurals? I am refering to line 463 of the diff below:
  return i18n.tr(FORMAT_LIMITS[i].formats[0], FORMAT_LIMITS[i].formats[1], val).arg(val)
Will the translation tool catch up with such a dynamic call to i18n.tr? Please note that I provide all the information necessary for plurals handling there.

If this is not supposed to work in *.js files I can prepare a QML component implementing this behaviour and reuse it in both: the calculator and the rssreader.

Cheers,
Dariusz

Revision history for this message
Roman Shchekin (mrqtros) wrote :

Thanks!

22.07.13 10:52 David Planella написал(а):

Al 22/07/13 08:34, En/na Roman Shchekin ha escrit:
> Yep, I think we must use "strong" realization rather than a liitle bit "naive" =)
>
> P.S. David, could your please have a look at my merge proposal? =)
> I can't provide you with url, but my branch is named stable-trunk-prepare2 as you may remember.
> Testing fails, I don't know why :(

Hi Roman,

I already commented on your other branch this morning.

Cheers,
David.

>
> 22.07.13 10:23 David Planella написал(а):
>
> Al 21/07/13 21:17, En/na Dariusz Gadomski ha escrit:
>> Hi,
>>
>> Thanks for you opinion. Could you please explain (or provide a link to any docs) how to improve it? I have only experience with pure (no gettext) Qt/QML apps for MeeGo utilizing qsTr in both: *.qml and *.js files. I did not have any clue how to implement i18n in *.js, that's why I pass i18n object there from QML.
>>
>> In case of MeeGo using an array of strings along with qsTr (like the in line 463 in the diff below) worked fine for all languages (all the culture-specific stuff was done by translators: e.g. inserting Right-To-Left characters etc.).
>>
>> Do you know any examples of i18n usage in *.js files?
>>
>> Thanks,
>> Dariusz
>>
>
> Also, I've just noticed that you're reimplementing a relative time
> calculation that is already done in the Calculator app.
>
>
> It might be worth looking at using just one way of calculating this.
> Looking at both at a glance, yours looks simpler and cleaner, but the
> calculator one handles translations and plurals for all languages.
>
>
> Do you think you could have a look at unifying the two functions?
>
>
> Cheers,
> David.
>

--
https://code.launchpad.net/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting/+merge/176051

Your team Ubuntu RSS Feed Reader Developers is requested to review the proposed merge of lp:~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting into lp:ubuntu-rssreader-app.

Revision history for this message
Joey Chan (qqworini) wrote :

u may try ".import Ubuntu.Components 0.1" in a .js file, i18n will be included

Revision history for this message
Roman Shchekin (mrqtros) wrote :

Joey is right, use ".import ..." Syntax in *.js files.

22.07.13 11:20 Joey Chan написал(а):

u may try ".import Ubuntu.Components 0.1" in a .js file, i18n will be included

--
https://code.launchpad.net/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting/+merge/176051

Your team Ubuntu RSS Feed Reader Developers is requested to review the proposed merge of lp:~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting into lp:ubuntu-rssreader-app.

Revision history for this message
David Planella (dpm) wrote :

On Mon, Jul 22, 2013 at 9:05 AM, Dariusz Gadomski <email address hidden>wrote:

> I would love to improve my proposal, but I still miss some info.
>
> In the calculator app I can see the internationalization implemented in
> Storage.qml file as part of the Storage component. I wanted to have it
> isolated in a *.js library in rssreader. I was not able to find out how to
> use i18n object properly in *.js files, that is why I pass 'i18n' object
> form QML to JS and use it there.
>
>
I think what you're trying to do makes sense, only that I've never used the
i18n functions outside of QML files. You might want to try what Joey is
mentioning in his last comment.

> David: do you think it will not work in terms of
> internationalization/plurals? I am refering to line 463 of the diff below:
> return i18n.tr(FORMAT_LIMITS[i].formats[0],
> FORMAT_LIMITS[i].formats[1], val).arg(val)
> Will the translation tool catch up with such a dynamic call to i18n.tr?
> Please note that I provide all the information necessary for plurals
> handling there.
>
>
That's correct, I believe that'd work at runtime. However, what will not
work there are the tools that extract the translations from the code to
expose them to translators. Being more specific, I don't think xgettext
would work here, as it expects the translatable strings to be wrapped in
i18n.tr() calls.

Also note that while we want to start marking all strings for translation,
the RSS reader has not yet been set up for internationalization (which we
need to do as soon as we land some of the current big changes). I'm
mentioning this because if you want to test i18n, it might be necessary to
set it up first.

> If this is not supposed to work in *.js files I can prepare a QML
> component implementing this behaviour and reuse it in both: the calculator
> and the rssreader.
>
>
Sounds like a good idea too. Let us know how it goes.

> Cheers,
> Dariusz
> --
>
> https://code.launchpad.net/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting/+merge/176051
> Your team Ubuntu RSS Feed Reader Developers is requested to review the
> proposed merge of
> lp:~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting into
> lp:ubuntu-rssreader-app.
>

Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

Ok, thanks. I will try to prepare a generic solution for both apps and setup i18n in rssreader to be able to test the solution.

Revision history for this message
David Planella (dpm) wrote :

Al 22/07/13 10:25, En/na Dariusz Gadomski ha escrit:
> Ok, thanks. I will try to prepare a generic solution for both apps and setup i18n in rssreader to be able to test the solution.
>

Sounds excellent, thanks Dariusz!

Essentially, what's needed to set up i18n for testing purposes is to add
this file adapted to the rss reader:

http://bazaar.launchpad.net/~ubuntu-weather-dev/ubuntu-weather-app/trunk/view/head:/po/po.pro

With it, you can run the 'make pot' rule on the command line to generate
the ubuntu-rss-reader-app.pot file. Generally, then the file is
committed and Launchpad takes care of exposing it to translators and to
create the .po files automatically, but for testing purposes you can
create a .po file manually from the .pot file.

There are two other steps required for translations to be loaded:

- Internationalization of the .desktop file adding the
  X-Ubuntu-Gettext-Domain key [1]
- Set up the Debian package to build and install the translation
  files [2]

But for manual, local testing you might not need these two.

[1]
http://bazaar.launchpad.net/~ubuntu-weather-dev/ubuntu-weather-app/trunk/view/head:/ubuntu-weather-app.desktop#L11
[2]
http://bazaar.launchpad.net/~ubuntu-weather-dev/ubuntu-weather-app/trunk/view/head:/debian/rules#L10

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches