fix(IconButton): Remove useLayoutEffect to avoid SSR warning #2831
Conversation
| '--ripple-scale-end': '1.414', | ||
| '--ripple-scale-start': '1.414', | ||
| '--ripple-size': '100%', | ||
| '--ripple-translate': '0, 0', |
There was a problem hiding this comment.
A square IconButton now uses the size prop instead of a measurement to make the ripple extend to the corners.
| @@ -24,9 +24,65 @@ | |||
|
|
|||
| */ | |||
|
|
|||
| import type { ExtendedStatefulColor } from '@looker/design-tokens' | |||
There was a problem hiding this comment.
Moved types from useRipple into this file.
| const [{ height, width }] = useMeasuredElement(element) | ||
| const result = useRipple({ ...props, bounded: true, height, width }) | ||
| return { ...result, ref } | ||
| } |
There was a problem hiding this comment.
Currently not used but will be for Button, ListItem, etc.
Coverage reportTotal coverage
Show new covered files 🌑Coverage of new files
Report generated by 🧪jest coverage report action from 2f05c88 |
|
|
||
| export const useBoundedRipple = (props: UseBoundedRippleProps) => { | ||
| const [element, ref] = useCallbackRef() | ||
| const [{ height, width }] = useMeasuredElement(element) |
There was a problem hiding this comment.
With an eye to the (not-too-distant) future do you think useMeasuredElement could be refactored to be more "defensive"? Basically just check if window exists and skip calculation if not?
I went ahead and logged a ticket to keep track of this requirement for that work: https://b.corp.google.com/issues/199953481 :)
There was a problem hiding this comment.
I think so. But in order to make that change, we should set up a test app that uses SSR. It sounds like neither useEffect nor useLayoutEffect should be used with SSR so I'm curious to see what falls apart with our components.
Using
ReactDOMServer.renderToString()on content containing anIconButtongenerates a warning foruseLayoutEffect: facebook/react#14927The
useLayoutEffecthook comes intoIconButtonviauseRipple->useMeasuredElement->useResize, but actually the last two hooks in that chain aren't necessary. I moved them to a separate hookuseBoundedRipplethat can be used for rectangular elements likeButtonandListItemwhere measurement is necessary.I did look into the
useIsomorphicLayoutEffectworkaround but it wouldn't work in the specific situation that inspired this ticket, becausewindowis still defined there.