Add support for rendering a custom wrapper around Element

[maheichyk]

Type: Enhancement
Related: https://github.com/matrix-org/matrix-react-sdk-module-api

This PR suggests to add possibility to add a wrapper around of Element that is configurable via module API.

This could be used to show custom header and footer.

It depends on PR to matrix-react-sdk-module-api: matrix-org/matrix-react-sdk-module-api#13

Checklist

• Tests written for new code (and old code if feasible)
• Linter and other CI checks pass
• Sign-off given on the changes (see CONTRIBUTING.md)

---

Here's what your changelog entry will look like:

✨ Features

• Add support for rendering a custom wrapper around Element (#25537). Contributed by @maheichyk.

[t3chguy]

Why is a banner preferable over toasts which are the direction the rest of the app design went? We ditched top app banners years ago in favour of toasts

[t3chguy]

CI is very unhappy, that'll need fixing before code review

Also not sure what benefit there is to this approach vs just injecting DOM elements or adding additional React roots.

[maheichyk]

Why is a banner preferable over toasts which are the direction the rest of the app design went? We ditched top app banners years ago in favour of toasts

It looks that toasts are a bit different, they are more like notifications with some action items. In our case we would like to have some region on the top that is always shown and includes some components: custom menu, logo, etc.

[t3chguy]

custom menu, logo, etc.

This sounds like better suited by adding your own React root or just injecting DOM elements, no?

[maheichyk]

custom menu, logo, etc.

This sounds like better suited by adding your own React root or just injecting DOM elements, no?

We would like to have a stable API that can be used for this feature. In addition, we want to put a component there that could toggle widgets in the Element (the idea here is to add more API changes to matrix-react-sdk-module-api to make the toggle possible). Not sure if multiple reacts roots fit into that.

[maheichyk]

CI is very unhappy, that'll need fixing before code review

This PR depends on matrix-react-sdk-module-api PR that is not merged yet. Maybe it is better to make this PR a draft.

[t3chguy]

Not sure if multiple reacts roots fit into that.

Sure, the buttons would be calling upon the Module API to perform actions either way, not like we'd be passing a context/props into the components from this end, given this end has no access to any such thing anyway.

[Johennes]

@maheichyk to reiterate @t3chguy's point, if all you want is a container at the top of the chat window, you should be able to inject that at the start of the body in your fork. That customization wouldn't depend on anything specific in Element Web or the React SDK. This means it's not possible for us to break it by changes in the FOSS code which then eliminates the need to create a stable API for it.

As @t3chguy says, if you want to trigger other functions from within the container, we should expose those in the module API so that you have a stable interface to rely upon.

Please let us know if there's another use case here that we haven't yet considered.

[dhenneke]

Our use case (I'm in the same team as @maheichyk btw) requires us to not do direct changes in a fork of Element so we would really prefer to have a modularized approach with a proper API.

Let me show an example module to see if I correctly understood your proposal:

export default class ExampleBannerModule extends RuntimeModule {
  public constructor(moduleApi: ModuleApi) {
    super(moduleApi);

    // ... register some lifecycles, add new translations, ...

    // this could also happen in the html file, but we prefer to not edit any upstream files in a fork
    const matrixRoot = document.getElementById("matrixchat");
    matrixRoot.style.height = "calc(100% - 64px)"; // say our banner has a height of 64px
    const newRoot = document.createElement("div");
    document.body.prepend(newRoot);

    // render a custom component. We could also pass the moduleApi to do translations or call any api
    ReactDOM.render(<Banner />, newRoot);
  }
}

There are some limitations, like that we can't know if and when Element was loaded correctly or if the user is logged in. But this could be provided by additional lifecycles/events in the Module API.

[t3chguy]

There are some limitations, like that we can't know if and when Element was loaded correctly or if the user is logged in.

Which the proposed changes to the Module API doesn't give you either, but you could subscribe to the matrix-react-sdk dispatcher to receive those events.

[dhenneke]

[...] subscribe to the matrix-react-sdk dispatcher to receive those events.

While possible, I think this is discouraged to import it directly into the module due to API stability reasons.

[t3chguy]

While possible, I think this is discouraged to import it directly into the module due to API stability reasons.

The alternative is an ever growing scope in the Module API as to what exact state the app is in. Right now you might only need to know when the user logs in, later the exact view they're in (login, registration, room, etc) later also more detail like they're doing verification as part of login, etc. Maintaining this is likely to be even more effort to not break between versions from the react-sdk PoV

[Johennes]

@dhenneke could you share more about what kind of additional data you'd require to be accessible from the banner? You already mentioned app loading and login state above. Are there any others that you're already aware of? Having a holistic view on what you're trying to achieve would help us to properly define the needed interface.

[dhenneke]

I think this banner use case is quite special in that regard. It could be an option to be a bit more flexible in this use case, but we will discuss this internally. The other use cases for the module system are usually more targeted and specific than this.

[dhenneke]

@Johennes currently we don't.

This is part of the effort to unify the UIs of different applications (e.g. Nextcloud) into a greater product suite. In general, it would also be nice to have the user-avatar menu in the top right, but the layout of Element is not really compatible with such a design that a lot of other tools use. So thats non-trivial and isn't really planned.

The banner would e.g. already be displayed when Element is still loading and that might be a bit too early:

element-banner-login.mov

But it might be fine for the initial version until we need more.

(don't worry about the look/layout of the banner in the video; there are reasons on why it is the way it is)

[Johennes]

Thanks for sharing @dhenneke.

The alternative is an ever growing scope in the Module API as to what exact state the app is in. Right now you might only need to know when the user logs in, later the exact view they're in (login, registration, room, etc) later also more detail like they're doing verification as part of login, etc. Maintaining this is likely to be even more effort to not break between versions from the react-sdk PoV

This is a fair concern. I wonder if we could mitigate it with an abstraction? Making high-level states like launching / launched available to modules seems plausible to me. But I agree that we probably wouldn't want to expose too granular states because then any change to them would require breaking the module API.

[maheichyk]

@germain-gg this is just a Draft PR and is basically just for information for now, because it seems that it cannot be built properly until we merge matrix-org/matrix-react-sdk-module-api#13 as it is complaining to missing WrapperLifecycle. So no need to review now, but thanks for the hint - I have requested review on that other PR in module-api.

[germain-gg]

Thank you for updating this! Couple of things that I believe need updating, but we're heading in the right direction

[germain-gg]

Approving, but also pending on the change you suggested doing.
Once done, I will merge this. Thank you 🙏

[germain-gg]

@maheichyk thank you for the updates. CI is slightly unhappy, therefore preventing me to merge this

[maheichyk]

@maheichyk thank you for the updates. CI is slightly unhappy, therefore preventing me to merge this

@germain-gg seems that there is a problem with the pipeline due to different react types comparing to matrix react sdk, I have asked in dev room as well.

[maheichyk]

@germain-gg CI is fine please have a look

Old stale review

[maheichyk]

@germain-gg many thanks for the review and merge!