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

[ Tech Debt] Replaced htmlIdGenerator with useGeneratedHtmlId #7346

Closed
wants to merge 1 commit into from

Conversation

cyberbuff
Copy link

Summary

Replaced htmlIdGenerator with useGeneratedHtmlId to support SSR. Fixes #7093

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Browser QA
    - [ ] Checked in both light and dark modes
    - [ ] Checked in mobile
    - [ ] Checked in Chrome, Safari, Edge, and Firefox
    - [ ] Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
    - [ ] Added documentation
    - [ ] Props have proper autodocs (using @default if default values are missing) and playground toggles
    - [ ] Checked Code Sandbox works for any docs examples
  • Code quality checklist
    - [ ] Added or updated jest and cypress tests
  • Release checklist
    • A changelog entry exists and is marked appropriately.
      - [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    - [ ] Updated the Figma library counterpart

Copy link

❌ Author of the following commits did not sign a Contributor Agreement:
ebead13

Please, read and sign the above mentioned agreement if you want to contribute to this project

Copy link

github-actions bot commented Nov 7, 2023

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

@tkajtoch
Copy link
Member

Hi @cyberbuff! Thank you for contributing to EUI and giving #7093 a shot!

Unfortunately, useHtmlIdGenerator is a React hook and cannot be used the same way as htmlIdGenerator was. There's way more work to it than just replacing all usages of htmlIdGenerator - you'd likely need to create HoCs that inject useHtmlIdGenerator to class components and figure out the most performant way to do so.

If you still want to work on the issue, please open a proof of concept PR first that showcases your approach to using that hook inside React class components. This way, we can confirm it looks right and passes our tests before you put more work into it.

I will close this draft PR for now since the solution is wrong and may confuse others.

@tkajtoch tkajtoch closed this Dec 21, 2023
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.

[SSR] Replace htmlIdGenerator usages with useGeneratedHtmlId in all function components
2 participants