Fix: Font size picker calls on change on each rerender.

[jorgefilipecosta]

Currently, the FontSizePicker component makes an onChange call each time the component is rerendered. The story I'm adding shows the issue in action.

This PR fixes the issue by making sure we are not generating new options on each rerender. This PR fixes part of the end to end tests failures on https://github.com/WordPress/gutenberg/pull/27594. Currently each time a block rerenders, the font size picker rerenders, and then we dispatch update block attribute actions.

I think that we also have an issue on the SelectDropdown component (it is the one throwing onChange calls), but I did not manage to replicate this issue using that component in isolation after multiple tries where on each rerender I passed different objects.

I verified using the developer tools and the story I proposed that on each rerender we are not making on Change calls.

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/itsjonq/g2/q7squus78
✅ Preview: https://g2-git-fork-jorgefilipecosta-fix-font-size-c-6fdc79.itsjonq.vercel.app

[ItsJonQ]

Haii! I made an update in this PR for the duplicate SelectDropdown callback: https://github.com/ItsJonQ/g2/pull/206

Would you be able to verify if it resolves the issue?

[jorgefilipecosta]

Hi @ItsJonQ, The issue is still present, I included a user story to test the issue http://localhost:6006/?path=/story/components-fontsizecontrol--periodic-rerender. If we revert: packages/components/src/font-size-control/font-size-control-utils.js packages/components/src/font-size-control/use-font-size-control.js

And just keep the story. We can see on the console that on each rerender on change is being called. So I guess we still need to merge this fix.

Regarding the select dropdown double callback, I think we still have part of the issue, on the same storybook during the first change from default to another value the double callback still happens. But now it is only the first time, in the changes after it does not happen.

[ItsJonQ]

@jorgefilipecosta Thanks for catching this 🙏 . I was able to identify the issue in SelectDropdown, specifically in the part where it syncs props and state value:

    // Sync incoming value.
    useUpdateEffect(() => {
        if (
            !store.getState().isOpen
        ) {
            store.getState().setCommitValue(value);
        }
    }, [value, store]);

I think FontSizeControl is passing in a new object (value) into SelectDropdown. This is because FontSizeControl remaps its options in getSelectOptions.

Although the shape of the value is identical, it fails the useEffect check because it's a new object.

The solution was to add an equality check in the if statement:

    useUpdateEffect(() => {
        if (
            !store.getState().isOpen &&
            !simpleEqual(store.getState().commitValue, value)
        ) {
            store.getState().setCommitValue(value);
        }
    }, [value, store]);

I think we can keep your memoized options update for extra safety.

---

I'm going to look adjusted getSelectOptions so that new objects aren't created every time

[ItsJonQ]

Update! Spreading the option in the options.map of getSelectOptions() helped!

    return options.map((option) => {
        const fontSize = is.number(option.size)
            ? createUnitValue(option.size, 'px')
            : option.size;

        return {
            ...option,
            key: option.slug,
            style: {
                fontSize: `min( ${fontSize}, ${MAX_FONT_SIZE_DISPLAY} )`,
            },
        };
    });