-
Notifications
You must be signed in to change notification settings - Fork 78
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
fix: MessageStrip Accessibility + useUniqueId hook #1156
Conversation
Deploy preview for fundamental-react ready! Built with commit e0d923d |
src/MessageStrip/MessageStrip.js
Outdated
@@ -47,17 +48,20 @@ const MessageStrip = (props) => { | |||
className | |||
); | |||
|
|||
const alertId = otherProps?.id || shortId.generate(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we create a useUniqueId()
hook like in widgets -
import shortid from './shortId';
import { useState } from 'react';
export default () => {
const [uniqueId, setUniqueId] = useState();
if (uniqueId) return uniqueId;
const id = shortid.generate();
setUniqueId(id);
return id;
};
src/MessageStrip/MessageStrip.js
Outdated
@@ -48,10 +48,10 @@ const MessageStrip = (props) => { | |||
className | |||
); | |||
|
|||
const alertId = otherProps?.id || shortId.generate(); | |||
const alertId = otherProps?.id || useUniqueId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't conditionally call hooks like this, or with ternary expressions like others.
We can split it onto separate lines though,
const generatedId = useUniqueId();
const alertId = otherProps?.id || generatedId;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚢
Description
In this change, we
aria-controls
value for theMessageStrip
close buttonuseUniqueId
hook to generate IDs in functional componentsfixes #1153
Before
After
Know Issues
https://github.com/SAP/fundamental-react/issues/1155