Closed Bug 1009134 Opened 8 years ago Closed 8 years ago

Transition away from using dataset.placeholder to localize placeholder

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(1 file)

Spin-off from bug 994290.

SMS is currently using a quirky feature of l10n.js to localize two input's placeholders.

We want to clean up node attribute localization and remove those quirks.

The code we're talking about is here:
 * https://github.com/mozilla-b2g/gaia/tree/e4d250f043e8d9334e3094103a46ccf1811a6114/apps/sms/locales/sms.en-US.properties#L43-L44
 * https://github.com/mozilla-b2g/gaia/blob/e4d250f043e8d9334e3094103a46ccf1811a6114/apps/sms/style/composer.css#L98-L104

I'm not sure if CSS will auto-update the value on language change, but I assume it will not.

If that's true, than moving away from dataset/CSS toward JS will also fix that.
Blocks: 994290
CC'ing the author of the code.

Borja, would you mind transitioning away from this toward JS solution?
Flags: needinfo?(borja.bugzilla)
(taking the NI because Borja merely moved this code)

> I'm not sure if CSS will auto-update the value on language change, but I assume it will not.

I'm pretty sure it does.


How do you propose to fix this in JS? Adding a "ready" handler?
And in that "ready" handler we'd do a querySelector on [data-l10n-id] like we already do for our time headers?
Flags: needinfo?(borja.bugzilla)
Attached file pull request
Yes!

Here comes the patch :)
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Attachment #8422706 - Flags: review?(felash)
Comment on attachment 8422706 [details] [review]
pull request

One question about making the code more generic, but otherwise it's a no brainer.

Keeping the r request for now.
Comment on attachment 8422706 [details] [review]
pull request

Removing the review request since we agreed on the path forward.
Please request review again once you're ready :)
Attachment #8422706 - Flags: review?(felash)
Comment on attachment 8422706 [details] [review]
pull request

Updated patch with js-l10n-placeholder and id->l10nId translation
Attachment #8422706 - Flags: review?(felash)
Comment on attachment 8422706 [details] [review]
pull request

r=me if you use Utils.camelCase instead of your own toCamelCase function.

I'm filing a bug for adding unit tests for this file (I understand you don't want to add unit tests for a file that's untested yet).
Attachment #8422706 - Flags: review?(felash) → review+
Bug 1012670 was created to track adding unit tests for startup.js
Switched to Utils.camelCase.

Patch: https://github.com/mozilla-b2g/gaia/commit/74635f4e507d00910cb1c11ffaf515fca112e6fb
Merge: https://github.com/mozilla-b2g/gaia/commit/3425e695233ff21a3054d088b5bef910f9751f12
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.