refactor: change Icon Picker architecture

[neatbyte-vnobis]

Changes

Refactor Icon Picker using "Fast Test Architecture".

Example of definition:

IconPackProvider

<IconPickerConfig>
    <IconPickerConfig.IconPack
        name={"emoji-icon-pack"}
        provider={() => [{type: "emoji", name: "thumbs_up", value: "👍"}]}
    />
</IconPickerConfig>

IconType

const EmojiIcon = () => {
    const { icon } = useIcon<Icon>();

    return <div>{icon.value}</div>;
};

const EmojiIconTab = observer(() => {
    const presenter = useIconPicker();
    
    const onIconSelect = (icon: Icon) => {
        presenter.setIcon(icon);
        presenter.closeMenu();
    };
    
    return (
        <IconPickerTab
            label={"Emojis"}
            onChange={onIconSelect}
        />
    );
});

<IconPickerConfig.IconType name={"emoji"}>
    <IconPickerConfig.IconType.Icon element={<EmojiIcon />} />
    <IconPickerConfig.IconType.Tab element={<EmojiIconTab />} />
</IconPickerConfig.IconType>

How Has This Been Tested?

Manual

Documentation

None

[neatbyte-vnobis]

@Pavel910

I've implemented the emoji tab following the icon tab implementation. However, there's an issue with the presenter value inside the plugins tabs components (EmojiTab, IconTab). It appears that they are only rendered once, and the presenter value from useIconPicker is assigned just once as well.

This is very noticable on the emoji tab, as SkinToneSelect does not receive the actual selectedIcon value.

On the icon tab it causes the first color change not being applied (the state changes, cells get rerendered via cellDecorator, but the actual icon value in the presenter is not changed). All subsequent color changes work, as the color state gets updated, triggering a rerender of IconTab, and the presenter is assigned a new value (although selectedIcon is still one state off, it's not as noticeable for IconColorPicker as it is for SkinToneSelect).

[Pavel910]

@neatbyte-vnobis I'll clone your branch and have a look a bit later.

[Pavel910]

@neatbyte-vnobis Check out my last commit:

• I removed the color from the Presenter, it's an implementation detail of the icon type, so it's the wrong place for that state.
• in the emoji type, I simplified the check for hasSkinToneSupport: you already have that information in the seletedIcon, so no need to dig through icons; this also eliminates the need for getIcons() on the Presenter.
• in general, Presenter should never be asked to give information (means, no getters should exist on the Presenter). Presenter provides data to the View (React component), via the vm. The only methods on the Presenter that are allowed, are the commands (setSomething, addSomething, updateSomething)
• the reason why the Tabs were not updated is because the presenter in the context is an observable and to make those components react to the changes, you ned to wrap those components into a observer() HOC. Basically, the rule is, every component that is reading data from an observable object, needs to be an observer. https://mobx.js.org/react-integration.html

I also left some comments in the code itself, so please see the commit.

[Pavel910]

Just tested your latest commit: works nice. And I believe you can see for yourself how nicely the plugins work now, without any changes to the core 👍

[neatbyte-vnobis]

@Pavel910

I've completed all the todos listed in this PR, except for the last one. I'am currently waiting for @leopuleo input on that particular item.

If everything else looks good, maybe we can merge this PR, so @SvenAlHamad can test if the UI works as expected.

[Pavel910]

Great progress, we're almost done. I left just a few more suggestions. I will also let @leopuleo and you polish out the exports of hooks.

Basically, the issue we're trying to solve is the flood of hooks being exported. Imagine if dozens of components had 5 hooks each, and you export all of them from the root of the package: there will sooner or later be name collisions. To export things in a meaningful way, we want to expose hooks and components not as top-level exports, but as properties of the component and its config. Hope this clarifies the goal.

[Pavel910]

Love it! 🥇

[neatbyte-vnobis]

@Pavel910 Is this PR ready to be merged?

[Pavel910]

@neatbyte-vnobis we still have components and hooks exports to finish. Do you have the necessary information from @leopuleo ?

[neatbyte-vnobis]

@neatbyte-vnobis we still have components and hooks exports to finish. Do you have the necessary information from @leopuleo ?

I've already made the following export updates:

1. Exported at the package root here.
2. Exported IconPickerTab like this.
3. Exported hooks here.

@leopuleo, based on these updates, do you think it is done, or is there something I might have missed?

[neatbyte-vnobis]

@leopuleo, please take a look at last question.
@Pavel910, @adrians5j, other than that, this one is ready for review.