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

Don't use Object.values in recordUtil #148

Merged
merged 1 commit into from Jul 18, 2017
Merged

Don't use Object.values in recordUtil #148

merged 1 commit into from Jul 18, 2017

Conversation

@ayumi
Copy link
Contributor

ayumi commented Jul 17, 2017

It doesn't work in iOS <= 10.2

Related to #147

It doesn't work in iOS <= 10.2

Fix #147
@ayumi ayumi requested review from diracdeltas and jhreis Jul 17, 2017
@ayumi ayumi added the ios label Jul 17, 2017
@@ -6,7 +6,8 @@ const serializer = require('../lib/serializer')
const valueEquals = require('../lib/valueEquals')

// ['0', '1', '2']
module.exports.CATEGORY_IDS = Object.values(proto.categories)
// webkit on iOS <= 10.2 does not support Object.values
module.exports.CATEGORY_IDS = Object.keys(proto.categories).map((key) => proto.categories[key])

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Jul 17, 2017

Member

it would be nice to make this either a global polyfill (if (!Object.values) {Object.values = ...}) or split it into a utility function since it is also used elsewhere (ebd7a1b)

This comment has been minimized.

Copy link
@ayumi

ayumi Jul 17, 2017

Author Contributor

if you set if (!Object.values) {Object.values = ...} can it apply to all files?

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Jul 17, 2017

Member

@ayumi if you add it to client/polyfill/ and then include it in index.html

This comment has been minimized.

Copy link
@ayumi

ayumi Jul 17, 2017

Author Contributor

@jhreis does that strategy work for you (adding an object.values polyfill to client/polyfill/)

@ayumi
Copy link
Contributor Author

ayumi commented Jul 17, 2017

I'm going to merge and leave #147 open until the third time we run into this / we figure out the Right way to do it

@ayumi ayumi merged commit d4d7d07 into staging Jul 18, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ayumi ayumi deleted the fix/object-values branch Jul 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.