Skip to content
This repository has been archived by the owner on May 22, 2021. It is now read-only.

refactored ftl file #269

Merged
merged 10 commits into from
Jul 25, 2017
Merged

refactored ftl file #269

merged 10 commits into from
Jul 25, 2017

Conversation

abhinadduri
Copy link
Collaborator

No description provided.

@dannycoates
Copy link
Contributor

ref #57

@dannycoates
Copy link
Contributor

@flodolo r?

Copy link
Collaborator

@flodolo flodolo left a comment

Choose a reason for hiding this comment

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

CC @Pike and @stasm

Can you add a l10n.toml file too as suggested in #57 (comment)?

More questions, because GitHub is a smart-ass and I can't comment on the entire file (didn't think of that):

  • Is "Firefox Send" a brand? Can it be localized? If not, we should probably not expose that string and hard code it.
  • "your stuff" in uploadPageExplainer: this sounds way too colloquial in a shipping product?

Final question: how final are these strings? Do they need to go through PM/Legal approval? If they do, we should wait before exposing them for localization.

@@ -7,7 +7,7 @@ uploadPageLearnMore = Learn more
uploadPageDropMessage = Drop your file here to start uploading
uploadPageSizeMessage = For the most reliable operation, it’s best to keep your file under 1GB
uploadPageBrowseButton = Select a file on your computer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you actually need the string on the entity, or only on the title attribute? If it's the former, this should be

uploadPageBrowseButton
    .title = Select a file on your computer

Same question for similar strings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need the string on the entity as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. @mathjazz is this going to work without issues in Pontoon?

@@ -19,66 +19,70 @@ notifyUploadDone = Your upload has finished.

uploadingPageMessage = Once your file uploads you will be able to set expiry options.
uploadingPageCancel = Cancel upload
.title = {uploadingPageCancel}
.title = Cancel upload
uploadCancelNotification = Your upload was cancelled.

uploadingPageLargeFileMessage = This file is large and may take awhile to upload. Sit tight!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pretty sure "awhile" -> "a while"


//Note the spec suggests that this string is editable. That feature will not appear at Launch
uploadSvgAlt
.alt = Upload
uploadSuccessTimingHeader = The link to your file will expire after 1 download or in 24 hours.
copyUrlFormLabel = Copy and share the link to send your file:
copyUrlFormLabelWithName = Copy and share the link to send your file: { $filename }
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the difference between these two strings?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you show me a screenshot of these two strings? I'm not sure I understand how and when they would be used, e.g. the first has a colon because it's followed by something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

copyUrlFormLabel is only being used in one place. I refactored that line so we can safely remove this string now.


downloadAltText.alt = Download
downloadAltText
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this an alt text on a button or link? We need a comment to explain it's an action, not a noun

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's an alt text on an svg element. I'll go ahead and add in a comment.


downloadAltText.alt = Download
downloadAltText
.alt = Download
downloadFileName = Download { $filename }
downloadFileSize = ({ $size })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where does size come from and how it's formatted?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The file size comes in from the server and is rendered through handlebars to the ftl file. It's a string in a human readable format, i.e. 12 Mb.

downloadFileName = Download { $filename }
downloadFileSize = ({ $size })
downloadMessage = Your friend is sending you a file with Firefox Send, a service that allows you to share files with a safe, private, and encrypted link that automatically expires to ensure your stuff does not remain online forever.
downloadButtonLabel = Download
.title = {downloadButtonLabel}
.title = Download
downloadNotification = Your download has completed.
downloadFinish = Download Complete

sendYourFilesLink = Try Firefox Send
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs a note in case Firefox Send should not be localized

errorPageHeader = Something went wrong!
errorPageMessage = There has been an error uploading the file.
errorPageLink = Send another file


linkExpiredAlt.alt = Link expired
linkExpiredAlt
.alt = Link expired
expiredPageHeader = This link has expired or never existed in the first place!
notSupportedHeader = Your browser is not supported.
notSupportedDetail = Unfortunately this browser does not support the web technology that powers Firefox Send. You'll need to try another browser. We recommend Firefox!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: proper apostrophe instead of straight quote.

Note about Firefox Send if needed.


uploadedFile = File
copyFileList = Copy URL
expiryFileList = Expires In
Copy link
Collaborator

Choose a reason for hiding this comment

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

How is this string used? It smells of contenation

uploadedFile = File
copyFileList = Copy URL
expiryFileList = Expires In
deleteFileList = Delete
nevermindButton = Nevermind
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Never mind"?

@abhinadduri
Copy link
Collaborator Author

@flodolo Is there any documentation on the l10n.toml file you're mentioning? I'm looking at the linked toml file and I'm not too sure how to translate it to Send. Thanks!

@flodolo
Copy link
Collaborator

flodolo commented Jul 21, 2017

http://moz-l10n-config.readthedocs.io/en/latest/fileformat.html

It should be basically identical, with public/locales instead of locales, and I guess we can drop the [locales] section for now.

@abhinadduri
Copy link
Collaborator Author

@flodolo I have added in the requested changes and the toml file. Please let me know if there are any more changes I need to make. Thanks.

downloadButtonLabel = Download
.title = Download
downloadNotification = Your download has completed.
downloadFinish = Download Complete

// We might not need to localize this if it's a brand name.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to answer this question and a comment with the answer ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops... @wresuolc what are your thoughts on this?

@@ -43,16 +44,21 @@ deleteFileButton = Delete file
sendAnotherFileLink = Send another file
.title = Send another file

// This alt text and the download filename string are actions.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternative text used on the download link/button (indicates an action).

downloadFileSize = ({ $size })
downloadMessage = Your friend is sending you a file with Firefox Send, a service that allows you to share files with a safe, private, and encrypted link that automatically expires to ensure your stuff does not remain online forever.

// This is also an action.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternative text used on the download link/button (indicates an action).

(localizers don't see the whole file, only one string at a time)

@flodolo
Copy link
Collaborator

flodolo commented Jul 21, 2017

What about the question on copy/legal review? Also, as explained in a comment, you need to answer the question "Is Firefox Send a brand name and should not be localized?".

sentFilesTitle4 = Delete
uploadedFile = File
copyFileList = Copy URL
expiryFileList = Expires In
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this string followed by?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Expires In is a column header. It isn't followed by anything, but there are times listed below it (one for each file).

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, can you add a comment explaining that? As simple as

// expiryFileList is used as a column header

P.S. I'm in EU timezone, I'll probably catch up next week with this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure thing. Thanks for all the suggestions!

@abhinadduri
Copy link
Collaborator Author

I have added comments stating that Firefox Send and Test Pilot do not be localized, as they are proper names.

Copy link

@Pike Pike left a comment

Choose a reason for hiding this comment

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

Thanks for the l10n.toml, the file works great.

For send.ftl, we'll need some formatting changes still, please make sure that https://github.com/projectfluent/python-fluent/blob/master/tools/fluentfmt.py round-trips your ftl file without a change.

Copy link
Collaborator

@flodolo flodolo left a comment

Choose a reason for hiding this comment

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

@Pike do we care about blank lines between strings?

@@ -19,68 +20,82 @@ notifyUploadDone = Your upload has finished.

uploadingPageMessage = Once your file uploads you will be able to set expiry options.
uploadingPageCancel = Cancel upload
.title = {uploadingPageCancel}
.title = Cancel upload
Copy link
Collaborator

Choose a reason for hiding this comment

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

4 spaces indentation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

downloadAltText.alt = Download
// Alternative text used on the download link/button (indicates an action).
downloadAltText
.alt = Download
downloadFileName = Download { $filename }
Copy link
Collaborator

Choose a reason for hiding this comment

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

One space before =

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, thanks!

@Pike
Copy link

Pike commented Jul 23, 2017

@Pike do we care about blank lines between strings?

I'm afraid we do right now.

@flodolo
Copy link
Collaborator

flodolo commented Jul 24, 2017

A few more questions that I need answered before opening up the project on Pontoon:

  • Is there a stage/dev server? Will it be updated automatically with updated localizations? How often?
  • Are there deadlines?
  • How often will production be updated with new localizations?

@abhinadduri
Copy link
Collaborator Author

@dannycoates Any idea on localization deadlines and update times?

@abhinadduri
Copy link
Collaborator Author

@flodolo So should I remove all line breaks from the ftl file?

@flodolo
Copy link
Collaborator

flodolo commented Jul 24, 2017

@flodolo So should I remove all line breaks from the ftl file?

Empty lines, yes. You can try running the script above on your FTL file
https://github.com/projectfluent/python-fluent/blob/master/tools/fluentfmt.py

@dannycoates
Copy link
Contributor

We have no deadlines since we can deploy as translations are completed.

dev site: https://send.dev.mozaws.net (updated automatically on each push to master)
stage site: https://send.stage.mozaws.net (updated automatically on version tag)
production site: https://send.firefox.com (updated manually after stage)

We're able to deploy translations to production at any time, but likely with new versions once a week.

@flodolo
Copy link
Collaborator

flodolo commented Jul 24, 2017

Can you also add write access to https://github.com/mozilla-pontoon?

Once the file has been updated and merged, I'll try setting up Pontoon on stage tomorrow and check that everything works as expected.

@dannycoates
Copy link
Contributor

Can you also add write access to https://github.com/mozilla-pontoon?

done 👍

@abhinadduri
Copy link
Collaborator Author

@flodolo I removed the empty lines and am running the script, which seems to be outputting the same ftl as the input file. However, there's also an extra newline at the end of the python program's output: will this be an issue?

@flodolo
Copy link
Collaborator

flodolo commented Jul 24, 2017

Let's keep the same output, so empty line at the end of the file.

@flodolo
Copy link
Collaborator

flodolo commented Jul 24, 2017

One more note: once we start localizing, rule for string changes is the same as Firefox
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

@flodolo flodolo mentioned this pull request Jul 25, 2017
Copy link
Collaborator

@flodolo flodolo left a comment

Choose a reason for hiding this comment

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

Please use a single … instead of ... (3 dots)

fileTooBig = That file is too big to upload. It should be less than { $size }.

linkExpiredAlt.alt = Link expired
Copy link
Collaborator

@flodolo flodolo Jul 25, 2017

Choose a reason for hiding this comment

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

This needs to be updated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just fixed this!

@abhinadduri
Copy link
Collaborator Author

@flodolo I've added in the requested changes above. The latest push has an ftl file that matches the output of running fluentfmt.py on the file. Please let me know if there are more changes that need to be made before this merges into maser. Thanks so much!

Copy link
Collaborator

@flodolo flodolo left a comment

Choose a reason for hiding this comment

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

Let's merge this one, so I can test Pontoon (not sure I'll manage today, tomorrow is more likely).

@abhinadduri abhinadduri merged commit 1bfa632 into master Jul 25, 2017
@dannycoates dannycoates deleted the ftlFixes branch July 31, 2017 21:38
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