fix(animated-fab): label styling (web) #4567
Conversation
|
@BogiKay is this able to be merged in soon? I'd like to use this component on our web app |
I'll take care of it. I'll do my best to have it merged by the end of next week. |
| @@ -428,3 +435,36 @@ export const getExtendedFabStyle = ({ | |||
|
|
|||
| return isV3 ? v3Extended : extended; | |||
| }; | |||
|
|
|||
| const getCanvasContext = () => { | |||
There was a problem hiding this comment.
I'm not expert on web, won't impact the performance creating everytime a canvas? will work cache it? something like
let cachedCanvasContext: CanvasRenderingContext2D | null = null;
const getCanvasContext = () => {
if (!cachedCanvasContext) {
const canvas = document.createElement('canvas');
cachedCanvasContext = canvas.getContext('2d');
}
return cachedCanvasContext;
};
There was a problem hiding this comment.
Valid suggestion
I did some performance testing for 100 FAB component instances rendered and TBH I didn't notice significant difference between variant with caching and the one without it except little memory consumption improvement, but still it's improvement, so caching added. 💪
|
This looks good to me! Is it able to be merged? It's one of a couple of issues holding up a new web launch for my team. |
Motivation
Currently, whole styling related logic in AnimatedFAB component relies on
onTextLayout, which is mobile specific, so FAB's label is not styled well in web.Related issue
#4478
Test plan