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

[TextField] Fix server-side rendering #4260

Closed
wants to merge 1 commit into from
Closed

[TextField] Fix server-side rendering #4260

wants to merge 1 commit into from

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented May 13, 2016

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

Closes #3757.
Closes #4253.

I have added some tests. I should have added them the first time. That would have same me this PR 😁 .

@AJIH What do you think about this change instead?

@ajihyf
Copy link
Contributor

ajihyf commented May 14, 2016

This PR is more clear but requires manually setting unique name or id. #4253 is a little hacky but supports generating non-conflict ids in both client and server sides. It depends on your choice to trade off. 😉

@alitaheri
Copy link
Member

Finding an id for every used TextField in an app only to support ssr seems overkill to me. I think @AJIH's method, though hacky is easier to deal with. Another approach would be, not to generate any id and only set id and htmlFor when the user provides an id, instead of auto-generating one if they don't. I think: "if you want SSR give id to every TextField" is less convenient than "if you want ARIA provide id to important TextFields"

@oliviertassinari
Copy link
Member Author

@alitaheri The id is not only here for ARIA, it's also to make the floating label focusing the input.
I'm gonna close this PR. @AJIH method is more resilient and I can't find real edge cases.

@oliviertassinari oliviertassinari deleted the unique-id-fix branch May 14, 2016 09:37
@alitaheri
Copy link
Member

@oliviertassinari Ops 😅 😅 sorry about that, I didn't know.

@oliviertassinari
Copy link
Member Author

@AJIH @alitaheri Thanks for the input 👍.

@ajihyf ajihyf mentioned this pull request May 18, 2016
3 tasks
@zannager zannager added the component: text field This is the name of the generic UI component, not the React module! label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: text field This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TextField] uniqueId breaks server rendering
4 participants