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

[TIMOB-25790] Do not override global L function to prevent error on iOS #109

Merged
merged 2 commits into from Mar 31, 2018
Merged

[TIMOB-25790] Do not override global L function to prevent error on iOS #109

merged 2 commits into from Mar 31, 2018

Conversation

ewanharris
Copy link
Contributor

@ewanharris ewanharris commented Feb 27, 2018

https://jira.appcelerator.org/browse/TIMOB-25790

Verification

  1. Set up a project with i18n folders (or just grab https://github.com/ewanharris/TIMOB-25790, note that if your locale isn't 'en' you might need to copy the files to the proper folder)
  2. Build the application with liveview
  3. Edit the enter_email value in i18n/en/strings.xml and save the file

Expected

When the app is reloaded the label should show the value you changed it to

@sgtcoolguy sgtcoolguy self-requested a review February 28, 2018 18:47
Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

LGTM. You could move the L function to hang it off Module, like the overridden require is, so that it isn't a nested function within Module.prototype._compile. Not sure if that would matter much, but I assume it may be more performant.

@ewanharris
Copy link
Contributor Author

@sgtcoolguy Yeah you're right, it's definitely in a bad-ish place right now and doesn't need to be defined every time we call compile for a script. I'll define it elsewhere, not sure about hanging off Module though as it's not really related to the Module functionality like require

@hansemannn hansemannn merged commit a5ee644 into tidev:master Mar 31, 2018
@ewanharris ewanharris mentioned this pull request Jun 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants