Implement TextualEvent tiles for im.vector.modular.widgets #1312
Conversation
E.g. "Bob added a Acme widget", "Susan removed a Giraffe widget"
E.g. "Bob added a Acme widget", "Susan removed a Giraffe widget" The name is calculated by taking the `name` in the event content, falling back on the `type`, falling back on the previous content `type`. This is then capitalised.
…ix-org/matrix-react-sdk into luke/feature-widget-timeline-events
src/TextForEvent.js
Outdated
| @@ -248,6 +248,26 @@ function textForPowerEvent(event) { | |||
| }); | |||
| } | |||
|
|
|||
| function textForWidgetEvent(event) { | |||
| const senderName = event.sender ? event.sender.name : event.getSender(); | |||
| const previousContent = event.getPrevContent() ? event.getPrevContent() : {}; | |||
There was a problem hiding this comment.
I don't know how costly a called to getPrevContent() is, but in JavaScript I think the idiom is:
const previousContent = event.getPrevContent() || {};
There was a problem hiding this comment.
oh. absolutely
src/TextForEvent.js
Outdated
| function textForWidgetEvent(event) { | ||
| const senderName = event.sender ? event.sender.name : event.getSender(); | ||
| const previousContent = event.getPrevContent() ? event.getPrevContent() : {}; | ||
| const {name, type, url} = event.getContent() ? event.getContent() : {}; |
There was a problem hiding this comment.
Same here: const {name, type, url} = event.getContent() || {};
src/TextForEvent.js
Outdated
| let widgetName = widgetName || name || type || previousContent.type || ''; | ||
|
|
||
| // Apply sentence case | ||
| widgetName = widgetName ? widgetName[0].toUpperCase() + widgetName.slice(1).toLowerCase() + ' ' : ''; |
There was a problem hiding this comment.
Is there any good reason to lower-case the rest of the string? I think maybe preserving the case of the rest of the string is better. That is, just upper-case the first character and leave the rest as it is.
There was a problem hiding this comment.
This is the same logic applied to the app tile header, I was going for consistency. Ideally the name would be formatted as intended for human reading by Scalar itself.
src/TextForEvent.js
Outdated
| // Apply sentence case | ||
| widgetName = widgetName ? widgetName[0].toUpperCase() + widgetName.slice(1).toLowerCase() + ' ' : ''; | ||
|
|
||
| if (url) { |
There was a problem hiding this comment.
Technically if the widget state event has no content, then it has been removed, but I think this is sufficiently equivalent as having a url is required.
There was a problem hiding this comment.
I shall comment this, thanks
|
LGTM. |
This was caused by a broken assumption which was AppsDrawer component mounting === clicking on apps draw toggle. This was introduced in #1312. Known issue with this fix: deleting the last app doesn't hide the app drawer.
E.g. "Bob added a Acme widget", "Susan removed a Giraffe widget" The name is calculated by taking the
namein the event content, falling back on thetype, falling back on the previous contenttype. This is then capitalised.Should be squash merged.