CustomComponentLifecycle - Swap/Override components in element-web or matrix-react-sdk using module-api and the CustomComponentLifecycle

[JohnSimonsen]

CustomComponentLifecycle

This lifecycle borrows a lot from the existing WrapperLifecycle.ts(credit: Mikhail Aheichyk, Nordeck IT + Consulting GmbH.) and can be implemented in the same fashion. However we felt it was needed to create a seperate lifecycle as the intended usecase is different. The idea behind the CustomComponentLifecycle is that it should "swap" an existing standard element/matrix-react-sdk component with a custom component in it's place by writing a module. This deviates from the WrapperLifeCycle's usecase to wrap a component and append custom components as children to the wrapper / sibling of the wrapped component.

Changes

• Added CustomComponentLifecycle.ts
• Added CustomComponentLifecycle.test.tsx
• Updated types.ts - Added CustomComponentLifecycle to AnyLifecycle
• Updated README.md - Added description of CustomComponentLifecycle

Why is it needed?

• Code readability - A component wrapped in <customComponentOpts.CustomComponent></customComponentOpts.CustomComponent>, indicates that you have a lifecycle hook to access what happens with this component.
• One can easily deviate from the upstream with low-cost maintenance
• Opens a new door for people who wish to customise components
  • It is a "catch-all" case for customization, giving the developers access to customise any wrapped component using modules instead of forking the repository.
  • opens up for alternative open sourced components, which can be installed using build-config.

Implementation example

As an experiment / Proof of concept i attempted the following:
Can I swap the UserMenu component in matrix-react-sdk seamlessly, with a custom module with custom elements and behaviours/functionality. Still retaining the original state, children and reactivity?

• Yes! ✔

The highlighted elements are (customisations) proof of a "modified" UserMenu.
The reactivity from child to expand panel etc, are retained. And since I based my customisation on the existing standard UserMenu all the functionality was transfered (i.e light/dark theme, default menu options etc.)

1. The module custom-usermenu-module

I created a new Module called CustomUserMenuModule with the following dependencies

• Adding a dependency to matrix-react-sdk and matrix-js-sdk allowed me to harness the full type library and stores from those packages. Meaning I could copy the entire UserMenu.ts file from matrix-react-sdk to my module custom-usermenu-module without any errors. I could then make my customisations directly in my module.
• By having a dependency to the matrix-react-sdk you can create just as powerfull components as in a fork. Allowing you to re-use sub-components like buttons etc.
• The element-web dependency to the module is of course just an optional "build-config" dependency.

Implementation of the module:

export default class CustomUserMenuModule extends RuntimeModule {
    private readonly CustomComponent: CustomComponentOpts['CustomComponent'] = React.Fragment;
    public constructor(moduleApi: ModuleApi) {

        super(moduleApi);

        this.CustomComponent = function CustomComponent({children}){

            // Get the component the wrapper is containing / i.e get the component to be swapped.
            let usermenu: any = React.Children.toArray(children)[0]

            return(
             <CustomUserMenu isPanelCollapsed={usermenu.props.isPanelCollapsed}>
                {usermenu.props.children}
             </CustomUserMenu>
            )
        }

        this.on(CustomComponentLifecycle.UserMenu, this.onCustomComponent);
    }
    protected onCustomComponent: CustomComponentListener = (CustomComponentOpts) => {
        CustomComponentOpts.CustomComponent = this.CustomComponent;
    }
}

Note: Notice that the implementation uses the lifecycle proposed in this PR.

2. The Implementation example of the lifecycle in matrix-react-sdk (UserMenu)

📄matrix-react-sdk>src>components>views>SpacePanel.tsx
👇 (Lines with a "👈" are relevant in this implementation)

(...)
const SpacePanel: React.FC = () => {
    const [dragging, setDragging] = useState(false);
    const [isPanelCollapsed, setPanelCollapsed] = useState(true);
    const ref = useRef<HTMLDivElement>(null);
    useLayoutEffect(() => {
        if (ref.current) UIStore.instance.trackElementDimensions("SpacePanel", ref.current);
        return () => UIStore.instance.stopTrackingElementDimensions("SpacePanel");
    }, []);

    useDispatcher(defaultDispatcher, (payload: ActionPayload) => {
        if (payload.action === Action.ToggleSpacePanel) {
            setPanelCollapsed(!isPanelCollapsed);
        }
    });

    const isThreadsActivityCentreEnabled = useSettingValue<boolean>("threadsActivityCentre");

    const customUserMenuOpts: CustomComponentOpts = { CustomComponent: React.Fragment }; 👈-  Create wrapper
    ModuleRunner.instance.invoke(CustomComponentLifecycle.UserMenu, customUserMenuOpts) 👈 - Invoke ModuleRunner
    return (
        <RovingTabIndexProvider handleHomeEnd handleUpDown={!dragging}>
            {({ onKeyDownHandler, onDragEndHandler }) => (
                <DragDropContext
                    onDragStart={() => {
                        setDragging(true);
                    }}
                    onDragEnd={(result) => {
                        setDragging(false);
                        if (!result.destination) return; // dropped outside the list
                        SpaceStore.instance.moveRootSpace(result.source.index, result.destination.index);
                        onDragEndHandler();
                    }}
                >
                    <nav
                        className={classNames("mx_SpacePanel", { collapsed: isPanelCollapsed })}
                        onKeyDown={onKeyDownHandler}
                        ref={ref}
                        aria-label={_t("common|spaces")}
                    >
                        <customUserMenuOpts.CustomComponent> 👈 Wrap the component to be "override-able"
                            <UserMenu isPanelCollapsed={isPanelCollapsed}>
                                <AccessibleTooltipButton
                                    className={classNames("mx_SpacePanel_toggleCollapse", { expanded: !isPanelCollapsed })}
                                    onClick={() => setPanelCollapsed(!isPanelCollapsed)}
                                    title={isPanelCollapsed ? _t("action|expand") : _t("action|collapse")}
                                    tooltip={
                                        <div>
                                            <div className="mx_Tooltip_title">
                                                {isPanelCollapsed ? _t("action|expand") : _t("action|collapse")}
                                            </div>
                                            <div className="mx_Tooltip_sub">
                                                {IS_MAC
                                                    ? "⌘ + ⇧ + D"
                                                    : _t(ALTERNATE_KEY_NAME[Key.CONTROL]) +
                                                      " + " +
                                                      _t(ALTERNATE_KEY_NAME[Key.SHIFT]) +
                                                      " + D"}
                                            </div>
                                        </div>
                                    }
                                />
                            </UserMenu>
                        </customUserMenuOpts.CustomComponent> 👈 Close customUserMenu wrapper
                        <Droppable droppableId="top-level-spaces">
                            {(provided, snapshot) => (
                                <InnerSpacePanel
                                    {...provided.droppableProps}
                                    isPanelCollapsed={isPanelCollapsed}
                                    setPanelCollapsed={setPanelCollapsed}
                                    isDraggingOver={snapshot.isDraggingOver}
                                    innerRef={provided.innerRef}
                                >
                                    {provided.placeholder}
                                </InnerSpacePanel>
                            )}
                        </Droppable>

                        {isThreadsActivityCentreEnabled && (
                            <ThreadsActivityCentre displayButtonLabel={!isPanelCollapsed} />
                        )}
                        <QuickSettingsButton isPanelCollapsed={isPanelCollapsed} />
                    </nav>
                </DragDropContext>
            )}
        </RovingTabIndexProvider>
    );
};
(...)

Note: Notice that the implementation uses the lifecycle proposed in this PR.

Signed-off-by: John Tore Simonsen jts@verji.com

[JohnSimonsen]

@MidhunSureshR @t3chguy
I am having trouble requesting a code reviewer.
Could any of you help me out?

[JohnSimonsen]

@robintown @t3chguy
Could I have another go at the workflow? I think the lint-errors should be fixed now.

[robintown]

I personally don't have a lot of context for how this project works, and to what extent it already embraces things that feel a little hacky, but that's my impression of this solution.

Am I right that this only allows customization of class components? While many important components in matrix-react-sdk are classes, nowadays we're trying to only write functional components, so I'd much prefer a solution that generalizes to them as well. If it's not possible to customize functional components in a similar manner, then adding this feature could create a counter-productive incentive to rewrite our functional components as classes.

I'll need to defer final judgement to someone like @t3chguy who may have more experience with the module API, but hopefully my thoughts are of some use.

[robintown]

All of this casting and use of any feels not great. Is there some way we can get more type-safety when customizing like this? (again this goes back to not knowing how much type-safety to expect in this project)

[JohnSimonsen]

Just to be clear, you are referring to:

1. super(undefined as any);
   and
2. let usermenu: any = React.Children.toArray(children)[0]; ?

In case 1. seems to be the standard of all other lifecycle tests within this project. But I can look into it - should be possible to use the interface for ModuleApi instead.

In case 2. Is just a mock-implementation of what a module might look like. But I'll look into it to see if I can get rid of the explicit any. And see what I can do to provide more type-safety.

Appreciate the feedback @robintown! 👍

[JohnSimonsen]

I personally don't have a lot of context for how this project works, and to what extent it already embraces things that feel a little hacky, but that's my impression of this solution.

Am I right that this only allows customization of class components? While many important components in matrix-react-sdk are classes, nowadays we're trying to only write functional components, so I'd much prefer a solution that generalizes to them as well. If it's not possible to customize functional components in a similar manner, then adding this feature could create a counter-productive incentive to rewrite our functional components as classes.

I'll need to defer final judgement to someone like @t3chguy who may have more experience with the module API, but hopefully my thoughts are of some use.

In theory it shouldn't matter if it is a class or a functional component. As long as you are able to put the customComponentOpts wrapper around it. I haven't personally tried on a functional component, just yet. But i'll be happy to provide an example if necessary. I completely agree that handling customisations differently for different component types would be an anti-pattern.

I would greatly appreciate if you could also have a look at the example provided in the PR-description on how it could be implemented in matrix-react-sdk as well. If you / others in the Matrix team don't like the way the implementation looks - It would be good to catch it now, so we can adjust the lifecycle ahead of time.

As for some more context on the proposed solution: I did some alternative experimentation on how to achieve the end goal of component override/customisation(Factory implementations etc.).
But in my opinion I believe this was the best solution I found, as it has notable upsides:

• Low impact hook implementation in matrix-react-sdk
• Follows same structure as the WrapperLifecycle - which has already been accepted
• Would solve most customisation needs
• Ease of use / Versatile

That being said, I'd be willing to put down the work if there are any suggestions on how to improve it.

[t3chguy]

I'll need to defer final judgement to someone like @t3chguy who may have more experience with the module API, but hopefully my thoughts are of some use.

I have zero experience with the module API and very little context aside that so please do not block this one me.

[JohnSimonsen]

@robintown - I extended the tests to include usecase of how you can just as easily wrap and swap a functional component with the same method. I also took your comment into account and got rid off the casting in the constructor of module, and I dropped the "any", that was pointed out in one of the tests (Although was solved by casting the result. As React.Children.toArray() has multiple possible return values.)

If neither of you has experience with the module-api. Maybe I could suggest adding @germain-gg as a code reviewer?
By the looks of it he handled and accepted this PR: #13
This PR builds on the same ideas.

The mechanics of how this works, can be confusing.
And I realize that the PR-description is quite extensive;
I'll try to give a short and concise explanation, to ease the burden of mental gymnastics, when familiarizing with the concept.
(TL;DR)

1. Initialization of CustomComponentOpts:
const customComponentOpts: CustomComponentOpts = { CustomComponent: React.Fragment };

• We initialize customComponentOpts with React.Fragment, indicating an initial state where no modification is applied to the rendered output.

2. Invocation of ModuleRunner:
ModuleRunner.instance.invoke(CustomComponentLifecycle.UserMenu, customComponentOpts)

• The ModuleRunner is invoked with a lifecycle hook and customComponentOpts.
• If no module has registered a handler for the lifecycle. The wrapper remains an empty React.Fragment - containing original markup/component.
• However we can implement a handler in a module to replace the React.Fragment with a function.
• In this function we can extract state from it's children.
• And we gain the ability to update the contents of the wrapper itself - for example by returning a CustomComponent instead

This feature could help resolve reported issues, like:
#21

[langleyd]

I marked this as needs product/design as I think we would need their alignment/approval to understand if this is a way in which we want extensibility to work.

[JohnSimonsen]

@langleyd Understandable, thank you for getting back to me. Please let me know if I can be of assistance in any way.

Our team (Verji) have chosen to move ahead with this feature on our fork, as it provides us with a "cleaner", organised structure of custom code vs upstream, and drastically lessens the burden of updating/maintaining our fork.

[langleyd]

Hi @JohnSimonsen , This is a very generic solution and its somewhat unclear what the implications would be on our apps overall maintainability if we were to take this approach. It’s great that it helps you manage your fork but our product’s module apis(including on other platforms) are still experimental and without a more evolved strategy/guidance we can't accept this at this point in time.

[JohnSimonsen]

@langleyd Thanks for the update, a bit disappointed with the decision. It would've been a huge win to get this into the upstream, for us at least. And probably others, as it would allow the community to be able to create detailed customisations on component level, completely without having to fork the project.

We'll stay tuned if there are better solutions presented at a later time. In the meantime we'll get some mileage on this solution, and if you're interested - we'll be happy to give you an update later with more mature results.

Appreciate you took the time to look into it, @langleyd! 😊