feat(Tab): autogenerate IDs #7519
Conversation
|
✔️ Deploy preview for carbon-elements ready! 🔨 Explore the source changes: 5bf5685 🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-elements/deploys/600729c3a7267b00079a8428 😎 Browse the preview: https://deploy-preview-7519--carbon-elements.netlify.app |
|
✔️ Deploy preview for carbon-components-react ready! 🔨 Explore the source changes: 5bf5685 🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-components-react/deploys/600729c21f08630007ca59f8 😎 Browse the preview: https://deploy-preview-7519--carbon-components-react.netlify.app |
| @@ -105,9 +106,11 @@ export default class Tab extends React.Component { | |||
| onKeyDown: () => {}, | |||
| }; | |||
|
|
|||
| tabId = this.props.id || `tab-${getInstanceId()}`; | |||
There was a problem hiding this comment.
One quick question, we've gotten some feedback around DOM mismatches when using auto-ids and server-side rendering. Curious if there is something we could do here, or if we're just saying if you want to SSR then you should pass in an id 👀
Reference for the useId hook for background (totally get we can't use the hook in a class component): https://github.com/carbon-design-system/carbon/blob/master/packages/react/src/internal/useId.js#L8
There was a problem hiding this comment.
we're just saying if you want to SSR then you should pass in an
id
yeah this sounds logical to me, but how have we handled it previously? the auto-id generators can be found throughout the library currently so I thought it was a viable solution here
There was a problem hiding this comment.
@emyarod sounds great to me, especially for the class components 👍
andreancardona
added
status: ready to merge 🎉
and removed
status: ready for review 👀
labels
Closes #7511
This PR autogenerates tab IDs so that the prop remains optional but the functionality does not break if a value is not provided by the user
Changelog
Changed
Removed
Testing / Reviewing
Confirm that tabs function as expected when an ID is not provided