Skip to content
This repository was archived by the owner on Aug 9, 2023. It is now read-only.

Conversation

@kthjm
Copy link
Contributor

@kthjm kthjm commented Sep 27, 2017

In React only both data-* and alia-* are kebabCase, Otherwise all camelCase.
https://facebook.github.io/react/docs/dom-elements.html

So this purposes to output property names that is not included in property-information as camelCase not kebabCase. The properties is a few but includes viewBox. SVG can't live without viewBox, right?

I have already rewritten the test a bit, but I am not sure if it is correct.
Torned to PR between property-information and here but I had no courage to touch the former.

thanks

@kthjm
Copy link
Contributor Author

kthjm commented Oct 11, 2017

relate: rehypejs/rehype-react#5

I think the above issue is not SVG but React's attributeName problem.
So it can be resolved by output attributeNames that are out of property-information as camelCase instead of kebabCase.

@wooorm
Copy link
Member

wooorm commented Nov 17, 2017

Sorry for the wrong wait!

SVG support is a whole other issue (literally), could you update the test cases here to just use a custom prop? So that, if we start supporting SVG, we can add cases and support for other VDOMs as well?

@wooorm
Copy link
Member

wooorm commented Nov 17, 2017

And THANK YOU SO MUCH for working on this! 🎉

@wooorm wooorm changed the title Modify addAttribute to work attrribute that leak out from property-information Fix addAttribute for React Nov 17, 2017
@kthjm
Copy link
Contributor Author

kthjm commented Nov 23, 2017

I dont have confidence in interpretation I would like to ask.

Without changing any original test code (so after restoring), Add a tiny test case that depends only custom prop like:

t.test('should support `React.createElement` about attributes case', function (st) {})

Does this comply with your intention?

@kthjm
Copy link
Contributor Author

kthjm commented Nov 23, 2017

Oh may be I took misread, you mean that just add case to should support keys ?

@wooorm
Copy link
Member

wooorm commented Nov 24, 2017

Yes to your last comment!

@kthjm kthjm closed this Nov 24, 2017
@kthjm kthjm force-pushed the modify-addAttribute-react branch from fc29ee7 to ad9ecf1 Compare November 24, 2017 01:40
@kthjm
Copy link
Contributor Author

kthjm commented Nov 24, 2017

so sorry, I couldn't resolve non-fast-forward (I have deleted the branch by mistake) and rebuilding #9

@wooorm wooorm added ⛵️ status/released 🐛 type/bug This is a problem 👶 semver/patch This is a backwards-compatible fix 🗄 area/interface This affects the public interface labels Aug 11, 2019
@wooorm wooorm added the 💪 phase/solved Post is done label Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🗄 area/interface This affects the public interface 💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem

Development

Successfully merging this pull request may close these issues.

2 participants