Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2532783003:
[NTP] Translateable strings for recent tabs and physical web sections. (Closed)
Description[NTP] Translateable strings for recent tabs and physical web sections.
BUG=667764
Committed: https://crrev.com/c68f6585d9e6e195838d95334c63f42a5936a6a4
Cr-Commit-Position: refs/heads/master@{#441121}
Patch Set 1 #
Total comments: 1
Patch Set 2 : rebase and treib@ comment. #
Total comments: 4
Patch Set 3 : treib@ comments. #Patch Set 4 : rebase. #Patch Set 5 : "Open tabs" + some string comments improvements. #Patch Set 6 : rebase. #Patch Set 7 : added recent tab section empty string. #
Messages
Total messages: 39 (24 generated)
The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
vitaliii@chromium.org changed reviewers: + treib@chromium.org
Hi Marc, PTAL.
Code LGTM, but please wait with landing until the strings are agreed on, to avoid unnecessary translations of the non-final strings. https://codereview.chromium.org/2532783003/diff/1/components/ntp_snippets_str... File components/ntp_snippets_strings.grdp (right): https://codereview.chromium.org/2532783003/diff/1/components/ntp_snippets_str... components/ntp_snippets_strings.grdp:40: <message name="IDS_NTP_PHYSICAL_WEB_PAGE_SUGGESTIONS_SECTION_HEADER" desc="Header of the physical web section. The physical web section shows a list of physical web pages available near the user."> Seems like this could use a slightly longer explanation, otherwise translations might get weird (I wouldn't really know what a "physical web page" is, and chances are the translators won't either)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hi Marc, would you mind having another look at strings descriptions?
The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks! These are much better :) https://codereview.chromium.org/2532783003/diff/20001/components/ntp_snippets... File components/ntp_snippets_strings.grdp (right): https://codereview.chromium.org/2532783003/diff/20001/components/ntp_snippets... components/ntp_snippets_strings.grdp:44: <message name="IDS_NTP_PHYSICAL_WEB_PAGE_SUGGESTIONS_SECTION_EMPTY" desc="On the New Tab Page, text of the card explaining to the user that they can expect to see physical web page suggestions in this area in the future. Connection to the physical web section header (IDS_NTP_PHYSICAL_WEB_PAGE_SUGGESTIONS_SECTION_HEADER) must be clear, but the words (i.e. translation of 'nearby') may differ. The physical web section shows a list of physical web pages available near the user. A physical web page is a URL transmitted over bluetooth by a low power device (a beacon). The user must be at most 100 meters (generally even closer) away from the beacon to receive the URL. This allows the URL to be context based (e.g. one is at a train station and the schedule is available as a physical web page)."> I'm not sure if the IDs are generally visible to translators, so referencing IDS_NTP_PHYSICAL_WEB_PAGE_SUGGESTIONS_SECTION_HEADER might not actually help. (I'm not sure though; maybe finkm knows?) https://codereview.chromium.org/2532783003/diff/20001/components/ntp_snippets... components/ntp_snippets_strings.grdp:48: <message name="IDS_NTP_RECENT_TAB_SUGGESTIONS_SECTION_HEADER" desc="Header of the recent tabs section, which is a list of pages (tabs), which are currently opened and were automatically saved, displayed as cards on the New Tab Page."> were automatically saved for offline consumption?
Hi Marc, I addressed your comment, PTAL. https://codereview.chromium.org/2532783003/diff/20001/components/ntp_snippets... File components/ntp_snippets_strings.grdp (right): https://codereview.chromium.org/2532783003/diff/20001/components/ntp_snippets... components/ntp_snippets_strings.grdp:44: <message name="IDS_NTP_PHYSICAL_WEB_PAGE_SUGGESTIONS_SECTION_EMPTY" desc="On the New Tab Page, text of the card explaining to the user that they can expect to see physical web page suggestions in this area in the future. Connection to the physical web section header (IDS_NTP_PHYSICAL_WEB_PAGE_SUGGESTIONS_SECTION_HEADER) must be clear, but the words (i.e. translation of 'nearby') may differ. The physical web section shows a list of physical web pages available near the user. A physical web page is a URL transmitted over bluetooth by a low power device (a beacon). The user must be at most 100 meters (generally even closer) away from the beacon to receive the URL. This allows the URL to be context based (e.g. one is at a train station and the schedule is available as a physical web page)."> On 2016/12/06 10:07:06, Marc Treib wrote: > I'm not sure if the IDs are generally visible to translators, so referencing > IDS_NTP_PHYSICAL_WEB_PAGE_SUGGESTIONS_SECTION_HEADER might not actually help. > (I'm not sure though; maybe finkm knows?) Indeed, they are not visible. I added the value instead and a comment to change it if original string is changed. https://codereview.chromium.org/2532783003/diff/20001/components/ntp_snippets... components/ntp_snippets_strings.grdp:48: <message name="IDS_NTP_RECENT_TAB_SUGGESTIONS_SECTION_HEADER" desc="Header of the recent tabs section, which is a list of pages (tabs), which are currently opened and were automatically saved, displayed as cards on the New Tab Page."> On 2016/12/06 10:07:06, Marc Treib wrote: > were automatically saved for offline consumption? Done.
LGTM! (But still, please wait until the strings are final before landing)
Description was changed from ========== [NTP] Translateable strings for recent tabs and physical web sections. BUG=667764,668989 ========== to ========== [NTP] Translateable strings for recent tabs and physical web sections. BUG=667764 ==========
Patchset #5 (id:80001) has been deleted
The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
vitaliii@chromium.org changed reviewers: + jkrcal@chromium.org
Hi jkrcal@, could you have a look at string comments (i.e. descriptions)?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/01/02 07:26:08, vitaliii wrote: > Hi jkrcal@, > > could you have a look at string comments (i.e. descriptions)? lgtm (assuming the strings are final)
On 2017/01/02 08:58:33, jkrcal wrote: > On 2017/01/02 07:26:08, vitaliii wrote: > > Hi jkrcal@, > > > > could you have a look at string comments (i.e. descriptions)? > > lgtm (assuming the strings are final) Strings are confirmed with PM, but UI people may like to change them during the UI review, but it is unlikely for this to happen before FF. Therefore, I am landing these strings before FF.
The CQ bit was checked by vitaliii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Hi jkrcal@, Could you please have a look at IDS_NTP_RECENT_TAB_SUGGESTIONS_SECTION_EMPTY description? I am not sure how to ensure that "Open tabs" does not collide with the "Recent tabs [from other devices]".
On 2017/01/03 13:50:44, vitaliii wrote: > Hi jkrcal@, > > Could you please have a look at IDS_NTP_RECENT_TAB_SUGGESTIONS_SECTION_EMPTY > description? > > I am not sure how to ensure that "Open tabs" does not collide with the "Recent > tabs [from other devices]". This lgtm. I would strive for consistency with section title, here (the empty label is shown only rarely and only in combination with the section title). I am puzzled that you replace IDS_NTP_SUGGESTIONS_SECTION_EMPTY, is it not used anywhere else?
On 2017/01/03 14:06:30, jkrcal wrote: > On 2017/01/03 13:50:44, vitaliii wrote: > > Hi jkrcal@, > > > > Could you please have a look at IDS_NTP_RECENT_TAB_SUGGESTIONS_SECTION_EMPTY > > description? > > > > I am not sure how to ensure that "Open tabs" does not collide with the "Recent > > tabs [from other devices]". > > This lgtm. I would strive for consistency with section title, here (the empty > label is shown only rarely and only in combination with the section title). > > I am puzzled that you replace IDS_NTP_SUGGESTIONS_SECTION_EMPTY, is it not used > anywhere else? Yes, recent tabs and physical tab providers were the only users (at least according to code search) of this string.
The CQ bit was unchecked by vitaliii@chromium.org
The CQ bit was checked by vitaliii@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org Link to the patchset: https://codereview.chromium.org/2532783003/#ps140001 (title: "added recent tab section empty string.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1483452712679020,
"parent_rev": "e765962bb2b1b04f7f8e6c8dfed793aa54e8099c", "commit_rev":
"906e4d847ce4dcffd79856c2627915d408b8af7e"}
Message was sent while issue was closed.
Description was changed from ========== [NTP] Translateable strings for recent tabs and physical web sections. BUG=667764 ========== to ========== [NTP] Translateable strings for recent tabs and physical web sections. BUG=667764 Review-Url: https://codereview.chromium.org/2532783003 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== [NTP] Translateable strings for recent tabs and physical web sections. BUG=667764 Review-Url: https://codereview.chromium.org/2532783003 ========== to ========== [NTP] Translateable strings for recent tabs and physical web sections. BUG=667764 Committed: https://crrev.com/c68f6585d9e6e195838d95334c63f42a5936a6a4 Cr-Commit-Position: refs/heads/master@{#441121} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/c68f6585d9e6e195838d95334c63f42a5936a6a4 Cr-Commit-Position: refs/heads/master@{#441121} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
