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.

Sign up for GitHub

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

Jump to bottom

Warn when using hyphenated style property names #1662

Merged
merged 1 commit into from
Aug 11, 2014
Merged

Warn when using hyphenated style property names #1662

merged 1 commit into from
Aug 11, 2014
+110 −5

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Jun 8, 2014

Fixes #932

sophiebits
sophiebits reviewed Jun 9, 2014
View reviewed changes
src/browser/ui/dom/CSSPropertyOperations.js
if (__DEV__) {
var warnedStyleNames = {};

var warnHyphenatedStyleName = function (name) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: no space after function

@sophiebits
Copy link
Collaborator

Looks pretty good to me. After getting camelize synced out let's do camelizeStyleName instead of unhyphenateStyleName.

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 10, 2014

I updated the code.

@zpao
Copy link
Member

zpao commented Jun 17, 2014

camelize does exist already. I can sync it out. It's very similar to what you have here :)

zpao added a commit that referenced this pull request Jun 17, 2014
@zpao
Sync out camelize module
8bbbe90 
For use in #1662
zpao
zpao reviewed Jun 20, 2014
View reviewed changes
src/browser/ui/dom/__tests__/CSSPropertyOperations-test.js
beforeEach(function() {
require('mock-modules').dumpCache();
CSSPropertyOperations = require('CSSPropertyOperations');

mocks = require('mocks');
Copy link
Member

Choose a reason for hiding this comment

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

Do you need mocks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah right. I only found spyOn later and tried using mocks at first. Will remove.

@zpao zpao mentioned this pull request Jun 22, 2014
Merged
@sophiebits
Copy link
Collaborator

What do you think about also warning if someone writes Ms instead of ms (and possibly wrong casing of the other vendor prefixes as well)?

@zpao
Copy link
Member

zpao commented Jun 23, 2014

Seems reasonable, but let's do that in a second PR

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 24, 2014

@zpao I rebased it to use the synced camelize.

@zpao zpao self-assigned this Jun 24, 2014
@zpao zpao merged commit e625b8b into facebook:master Aug 11, 2014
zpao added a commit that referenced this pull request Aug 11, 2014
@zpao
Merge pull request #1662 from gaearon/warn-when-using-hyphenated-styl…
986f5a1 
…e-names

Warn when using hyphenated style property names

Closes #1662

Conflicts:
	src/browser/ui/dom/CSSPropertyOperations.js
@zpao
Copy link
Member

zpao commented Aug 11, 2014

Sorry this took so long to get in!

@gaearon gaearon deleted the warn-when-using-hyphenated-style-names branch August 11, 2014 18:31
@gaearon
Copy link
Collaborator Author

gaearon commented Aug 11, 2014

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@zpao zpao

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Warn when using hyphenated style values
3 participants
@gaearon @sophiebits @zpao