[EuiToast & Co.] Convert to function component, Emotion styling#6068
[EuiToast & Co.] Convert to function component, Emotion styling#6068thompsongl merged 15 commits intoelastic:mainfrom
Conversation
| // Toast dismissal side effect | ||
| // Ensure the callback has correct state by not enclosing it in `setTimeout` | ||
| useEffect(() => { | ||
| const toast = toastToDismiss; | ||
| // Because this is triggered by a setTimeout, and because React does not guarantee when | ||
| // state updates happen, it is possible to double-dismiss a toast | ||
| // including by double-clicking the "x" button on the toast | ||
| // so, first check to make sure we haven't already dismissed this toast | ||
| if (toast && toastIdToTimerMap.current.hasOwnProperty(toast.id)) { | ||
| dismissToastProp(toast); |
There was a problem hiding this comment.
Only important change in the conversion to using hooks.
Previsouly props.dismissToast was called directly from within the setTimeout function and that was fine. With hooks, the closure inside useCallback would "lock" the state at time of instantiation and state would be outdated by the time the callback occurred.
This could be solved by requiring consumers to use the previous state param in their state management (setToastList((prevToasts) => prevToasts.filter(() => {})); but we can avoid the requirement with this side effect.
| ${logicalCSS('padding-left', euiTheme.size.base)}; | ||
| ${logicalCSS('padding-right', euiTheme.size.base)}; | ||
| ${logicalCSS('padding-vertical', euiTheme.size.base)}; |
There was a problem hiding this comment.
Using -left and -right instead of -horizontal because these values are potentially overridden by :not(:empty) logic and -left/-right will not cancel -horizontal (different generated CSS properties).
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6068/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6068/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6068/ |
elizabetdev
left a comment
There was a problem hiding this comment.
Looks good @thompsongl! 🎉
I tested in Chrome, Firefox and Safari. Works well! 🚀
I just have two suggestions. Let me know what you think.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6068/ |
Summary
Converts
EuiGlobalToastListto a function component. Pretty straightforward, but see line comments for a more involved explanation.Converts
EuiToast,EuiGlobalToastListItemandEuiGlobalToastListto styling with@emotion.View by commit for the easiest review path!
Checklist