Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Editor] Update some strings (bug 1787299) #15378

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

calixteman
Copy link
Contributor

No description provided.

Comment on lines 262 to 266
editor_text_color=Color
editor_text_size=Size
editor_draw_color=Color
editor_draw_thickness=Thickness
editor_draw_opacity=Opacity
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, and elsewhere, why are we changing the string-ids when the content doesn't actually change?
Won't this cause localizers to (unnecessarily) have to re-translate this?

Can we please just revert these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was just to be consistent, i.e. avoid having some free_text and text.
I thought that adding a number was a bit ugly but if you're fine with that then I am too.

Comment on lines 254 to 257
editor_text.title=Text
editor_text_label=Text
editor_draw.title=Draw
editor_draw_label=Draw
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, and elsewhere, I'm not crazy about the new string-ids. Basically it seems preferable to me if the string-ids themselves are still more inline with the actual code, since without prior knowledge it'd not be immediately obvious that e.g. "editor_draw" refers to the Ink-annotation.

It's perhaps a bit less nice, but how about something like this (a pattern we already use elsewhere)?

Suggested change
editor_text.title=Text
editor_text_label=Text
editor_draw.title=Draw
editor_draw_label=Draw
editor_free_text2.title=Text
editor_free_text2_label=Text
editor_ink2.title=Draw
editor_ink2_label=Draw

@calixteman calixteman changed the title [Editor] Update some strings (bug 1788665) [Editor] Update some strings (bug 1787299) Sep 1, 2022
@Snuffleupagus
Copy link
Collaborator

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Sep 1, 2022

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/d1c1759a1ad6808/output.txt

@Snuffleupagus
Copy link
Collaborator

/botio integrationtest

@pdfjsbot
Copy link

pdfjsbot commented Sep 1, 2022

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.241.84.105:8877/7d462cfc3d7b59f/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Sep 1, 2022

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/78588cc35a1797e/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Sep 1, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/d1c1759a1ad6808/output.txt

Total script time: 2.24 mins

Published

@pdfjsbot
Copy link

pdfjsbot commented Sep 1, 2022

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/78588cc35a1797e/output.txt

Total script time: 6.83 mins

  • Integration Tests: FAILED

@pdfjsbot
Copy link

pdfjsbot commented Sep 1, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/7d462cfc3d7b59f/output.txt

Total script time: 5.13 mins

  • Integration Tests: Passed

@calixteman calixteman merged commit eab411a into mozilla:master Sep 2, 2022
@calixteman calixteman deleted the editor_strings branch September 2, 2022 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants