Support custom components for messages #26
Conversation
Half-Shot
changed the title
| /** | ||
| * Words to highlight on (e.g. from search results). | ||
| * May be undefined if the client does not need to highlight | ||
| */ | ||
| highlights?: string[]; | ||
| /** | ||
| * Should previews be shown for this event | ||
| */ | ||
| showUrlPreview?: boolean; |
There was a problem hiding this comment.
These seem a little arbitrary, why these but not e.g. highlightLink, editState?
I think the level of this abstraction may make it difficult if not impossible to edit events which have been "decorated" with e.g. your openproject module
There was a problem hiding this comment.
Admittedly this was completely random as I was building out the API, so probably need to think about what properties are interesting to modules. I don't want to just include all of them at the risk that we might remove them later, but some would be useful.
We can also take the approach here of just adding on-demand rather than doing all of them right now, which reduces our commitments.
There was a problem hiding this comment.
I think a shape where a lot are handled upstream e.g. url previews should just be a thing that's glued onto the bottom of the event, where the module could somehow inhibit the behaviour if it doesn't want url previews to be scraped out of the body rather than each module needing its own URL Preview renderer
There was a problem hiding this comment.
Okay, makes sense. I've included a mechanism where a custom component may override props for rendering the original component. Such that URL previews are now handled by Element, but can be disabled by the module.
As far as edited events go, it looks tricky to export that behaviour. To keep this simple for now I've just removed props that I'm not using for my demo module. I think we can be additive here and add new properties as they become important to users.
| * customComponents.registerMessageRenderer("m.room.message", (props, originalComponent) => { | ||
| * return <YourCustomComponent mxEvent={props.mxEvent} />; | ||
| * }); | ||
| * customComponents.registerMessageRenderer(/m\.room\.(topic|name)/, (props, originalComponent) => { | ||
| * return <YourCustomStateRenderer mxEvent={props.mxEvent} />; | ||
| * }); |
There was a problem hiding this comment.
Shouldn't the API distinct state vs not rather than being quite foot-gunny such that if the latter sees an m.room.name timeline event it'd render it?
There was a problem hiding this comment.
yeah, we could tighten up this example. I don't think we need a separate function for state, it's generally good practice within matrix to be checking state keys.
There was a problem hiding this comment.
In the spirit of the ongoing MVVM work I'd also be keen on simplifying the interface passed, passing entire js-sdk objects around is quite brittle and given we're in the talks of swapping out the js-sdk for the rust-sdk would be guaranteed to break
There was a problem hiding this comment.
I'm not sure what we can do here for the short term, to be honest. I feel like trying to factor MatrixEvent into some sort of common structure is going to have a slight performance overhead per event, and it just feels gross to maintain another structure that we'd have to keep in line with changes to the event structure.
I don't know about the rest of product here but I'd be more in favor of just having a breaking change when the rust-sdk pieces land to support whatever comes next.
| * ); | ||
| * ``` | ||
| */ | ||
| registerMessageRenderer( |
There was a problem hiding this comment.
This API has changed a bit again, to allow for message hints. The intention is that for some messages we do not want the user to be able to edit the message due to the changes that may have been made to the event tile. I've added a hints type that can be extended in the future to do more things, but for now tells Element never to show an edit UI for any event that matches our eventTypeOrFilter.
The other problem with the old iteration was that CustomMessageRenderFunction was not deterministic as it did some filtering inside of the function, and might have returned null to passthrough to another module or the base app. That's no longer the case, and instead the eventTypeOrFilter function/string is the only way to filter whether or not a module is in use. The advantage here is knowing which hints to pass back to the base app is now easier to figure out as we know which renderer will be used.
One thing to note that it is possible that mxEvent changes between being checked for message hints / v.s. rendering so there is a potential gap, but this is probably acceptable.
Half-Shot
force-pushed
the
hs/custom-component-api
branch
from
8e4056f to
953d029
Compare
|
CodeQL refuses to run on my fork, so reopening. |
2 participants
This adds support for defining custom components for rendering messages. The intention is that the module API holds a loose collection of strings/RegExps that map to a set of render functions. This allows injecting a custom component for specific message types / contents.
Element-side branch: https://github.com/element-hq/element-web/compare/hs/add-custom-component-modules
Example module: <hookshot PR>