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

Localization #245

Merged
merged 6 commits into from Jul 19, 2017
Merged

Localization #245

merged 6 commits into from Jul 19, 2017

Conversation

abhinadduri
Copy link
Collaborator

@abhinadduri abhinadduri commented Jul 19, 2017

I will fix merge conflicts after final review.

uploadSuccessConfirmHeader = Ready to Send


//Notethe spec suggests that this string is editable. That feature will not appear at Launch
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: add space between "Note" and "the"

</div>
<div class="description" data-l10n-id="downloadMessage"></div>
<img src="/resources/illustration_download.svg" id="download-img" alt="Download"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Translate "Download" alt attribute?

Copy link
Contributor

@dannycoates dannycoates left a comment

Choose a reason for hiding this comment

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

Looking awesome!

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.
<span id="dl-filename"
data-l10n-id="downloadFileName"
data-l10n-args='{"filename": "{{{filename}}}"}'></span>
Copy link
Contributor

Choose a reason for hiding this comment

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

use 2 braces for filename, {{filename}}

</div>
<div class="description" data-l10n-id="downloadMessage"></div>
<img src="/resources/illustration_download.svg" id="download-img" alt="Download"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

alt text should be localized too.

server/server.js Outdated
@@ -63,6 +65,9 @@ app.use(
app.use(busboy());
app.use(bodyParser.json());
app.use(express.static(STATIC_PATH));
app.use('/l20n', express.static(L20N));
app.use('/download/*/l20n', express.static(L20N));
app.use('/download/*/locales', express.static(LOCALES));
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need routes relative to /download?

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 express app doesn't recognize the static route if you don't explicitly add routes with respect to /download.

<div class="title">
This link has expired or never existed in the first place.
</div>
<div class="title" data-l10n-id="expiredPageHeader"></div>

<div class="share-window">
<img src="/resources/illustration_expired.svg" id="expired-img" alt="Link expired" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Translate alt?

@@ -47,8 +47,11 @@ $(document).ready(function() {
//on complete
Copy link
Collaborator

Choose a reason for hiding this comment

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

Disclaimer: I have zero idea what I'm talking about, but I'd assume that the filesizes may need to be localized as well. For example, up on lines 34-45 above where we say "23KB", or "199MB", or "1.5GB".

<a id="dl-firefox" href="https://www.mozilla.org/firefox/new/?scene=2" target="_blank">
<img src="/resources/firefox_logo-only.svg" id="firefox-logo" alt="Firefox"/>
<div id="dl-firefox-text">Firefox<br><span>Free Download</span></div>
<div id="dl-firefox-text">Firefox<br>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does "Firefox" need to be translated here?

<meta name="defaultLanguage" content="en-US">
<meta name="availableLanguages" content="en-US">
<link rel="localization" href="locales/send.{locale}.ftl">
<script defer src="/l20n/dist/web/l20n.js"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this handle rtl languages, or will we need to add special CSS somewhere?


<meta name="defaultLanguage" content="en-US">
<meta name="availableLanguages" content="en-US">
<link rel="localization" href="locales/send.{locale}.ftl">
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: leading / on /locales/... to be consistent with other absolute asset URLs.

@pdehaan
Copy link
Collaborator

pdehaan commented Jul 19, 2017

Also, now that we have /public/resources/send_logo_type.svg, do we need to have locale specific versions of that?

@abhinadduri abhinadduri merged commit ffc9386 into master Jul 19, 2017
@dannycoates dannycoates mentioned this pull request Jul 19, 2017
@dannycoates dannycoates deleted the localization branch July 31, 2017 21:39
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.

None yet

3 participants