Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve display of event subtypes #1283

Merged
merged 5 commits into from
Nov 2, 2022
Merged

improve display of event subtypes #1283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
99cd692
improve display of event subtypes
uhoreg Oct 12, 2022
089f1fe
add changelog
uhoreg Oct 12, 2022
d6345f0
apply changes from code review
uhoreg Oct 26, 2022
60d9518
pass the desired example name to the template
uhoreg Oct 28, 2022
d0d7bdb
apply changes from code review
uhoreg Nov 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/client_server/newsfragments/1283.clarification
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve display of event subtypes.
4 changes: 2 additions & 2 deletions content/client-server-api/modules/end_to_end_encryption.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ following error codes are used in addition to those already specified:
- `m.mismatched_commitment`: The hash commitment did not match.
- `m.mismatched_sas`: The SAS did not match.

{{% event event="m.key.verification.start$m.sas.v1" %}}
{{% event event="m.key.verification.start$m.sas.v1" title="`m.key.verification.start` with `method: m.sas.v1`" %}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to put this title in the definition itself? jsonschema defines a title attribute which seems like it would be ideal (https://json-schema.org/draft/2020-12/json-schema-validation.html#name-title-and-description)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mainly because some event definitions already use the title attribute (e.g. https://github.com/matrix-org/matrix-spec/blob/main/data/event-schemas/schema/m.room.power_levels.yaml) and set it to something other than what the current title is in the spec.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example you give looks like it should be the description rather than the title.

Given we know that none of the titles are rendered, maybe we just rip out the existing ones with no loss of value?

Sorry to make the job (even) bigger, but I feel like this is going to be a maintenance headache we should just solve rather than hack around for now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried doing this, and ran into a snag. All the events inherit (using allOf, usually indirectly) from core-event-schema/event.yaml, so I need to remove all the title properties from everything in the chain. However, removing the title property from event.yaml turns things like

image

becomes

image

☹️

The only way I can think of to actually get this to work the way we want it to is to modify all the event yaml files to have a title property set to the event type (e.g. title: m.room.message in m.room.message.yaml), which seem ... suboptimal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh what a pain in the butt.

It feels like really the problem here is that things like this shouldn't be referring to event.yaml anyway, and should use a more specific type (with an appropriate title).

image

(see also my complaints at #897)

... however, maybe fixing that right now is a bit out of scope and we should just stick with your original suggestion.


{{% event event="m.key.verification.accept" %}}

Expand Down Expand Up @@ -1145,7 +1145,7 @@ base64).

###### Verification messages specific to QR codes

{{% event event="m.key.verification.start$m.reciprocate.v1" %}}
{{% event event="m.key.verification.start$m.reciprocate.v1" title="`m.key.verification.start` with `method: m.reciprocate.v1`" %}}

#### Sharing keys between devices

Expand Down
2 changes: 1 addition & 1 deletion content/client-server-api/modules/instant_messaging.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ room itself such as a room name and topic.

#### Events

{{% event event="m.room.message" %}}
{{% event event="m.room.message" desired_example_name="m.room.message$m.text" %}}

{{% event event="m.room.name" %}}

Expand Down
2 changes: 1 addition & 1 deletion content/client-server-api/modules/server_notices.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ maximum. New connections are being refused by the server. What defines
"active" is left as an implementation detail, however servers are
encouraged to treat syncing users as "active".

{{% event event="m.room.message$m.server_notice" %}}
{{% event event="m.room.message$m.server_notice" title="`m.room.message` with `msgtype: m.server_notice`" %}}

#### Client behaviour

Expand Down
17 changes: 17 additions & 0 deletions layouts/partials/events/example.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{{/*

Renders an event example. Resolves `$ref`s, serializes as JSON, and ensures
that it can be included in HTML.

This partial is called with the example event object as its context.

*/}}

{{ $example_content := partial "json-schema/resolve-refs" (dict "schema" . "path" "event-schemas/examples") }}
{{ $example_json := jsonify (dict "indent" " ") $example_content }}
{{ $example_json = replace $example_json "\\u003c" "<" }}
{{ $example_json = replace $example_json "\\u003e" ">" | safeHTML }}

```json
{{ $example_json }}
```
28 changes: 14 additions & 14 deletions layouts/partials/events/render-event.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

Renders a single event, given:

* `event_name`: the name we want to display for the event
* `event_name`: the name to use for the event
* `event_data`: the event specification
* `desired_example_name` (optional): the exact name of the examples to render.
If `desired_example_name` is omitted we render all examples
whose names start with the `event_name`.
* `title` (optional): the title to display. May contain markdown. Defaults to
`event_name` wrapped in a <code> element.

*/}}

Expand All @@ -20,7 +22,7 @@
<summary>

<h1 id="{{ anchorize $event_name }}">
<code>{{ $event_name }}</code>
{{ with .title }}{{ . | markdownify }}{{ else }}<code>{{ $event_name }}</code>{{ end }}
</h1>

<hr/>
Expand Down Expand Up @@ -72,11 +74,16 @@ <h2>Examples</h2>
*/}}
{{ if $desired_example_name }}
{{ if eq $example_name $desired_example_name }}
{{ $example_content := partial "json-schema/resolve-refs" (dict "schema" $example "path" "event-schemas/examples") }}
```json
{{ jsonify (dict "indent" " ") $example_content }}
```
{{ partial "events/example" $example }}
{{ end }}
{{/*
If `$desired_example_name` is not given, we will include any
example that is equal to the event name. Normally, this would
be handled by the case below, but that case does not work if
the event name includes a "$".
*/}}
{{ else if eq $event_name $example_name }}
{{ partial "events/example" $example }}
{{/*
If `$desired_example_name` is not given, we will include any
examples whose first part (before "$") matches the event name
Expand All @@ -86,14 +93,7 @@ <h2>Examples</h2>
{{ $pieces := split $example_name "$" }}
{{ $example_base_name := index $pieces 0 }}
{{ if eq $event_name $example_base_name }}
{{ $example_content := partial "json-schema/resolve-refs" (dict "schema" $example "path" "event-schemas/examples") }}
{{ $example_json := jsonify (dict "indent" " ") $example_content }}
{{ $example_json = replace $example_json "\\u003c" "<" }}
{{ $example_json = replace $example_json "\\u003e" ">" | safeHTML }}

```json
{{ $example_json }}
```
{{ partial "events/example" $example }}
{{ end }}
{{ end }}
{{ end }}
Expand Down
21 changes: 17 additions & 4 deletions layouts/shortcodes/event.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,23 @@

This template is used to render an event.

It expects to be passed an `event` parameter, which is the name of a schema file under
"data/event-schemas/schema". The file extension is omitted. For example:
It takes the following parameters:

{{% event event="m.accepted_terms" %}}
* `event` (required): the name of a schema file under "data/event-schemas/schema".
The file extension is omitted. For example:

{{% event event="m.accepted_terms" %}}

* `desired_example_name` (optional): the name of the example file to use under
"data/event-schemas/examples", without the file extension. If omitted
defaults to the example file with the same name as the `event` parameter, and
(if the name does not contain a "$"), all examples that begin with the name
given by the `event` parameter followed by a "$". For example, if the
`event` parameter is "m.foo", then by default it will include the "m.foo"
example along with any examples starting with "m.foo$".

* `title` (optional): the title to use for the event. Defaults to the name
given in the `event` parameter.

This template replaces the old {{*_event}} template.

Expand All @@ -17,4 +30,4 @@
{{ $event_data = partial "json-schema/resolve-refs" (dict "schema" $event_data "path" $path) }}
{{ $event_data := partial "json-schema/resolve-allof" $event_data }}

{{ partial "events/render-event" (dict "event_name" .Params.event "event_data" $event_data)}}
{{ partial "events/render-event" (dict "event_name" .Params.event "event_data" $event_data "desired_example_name" .Params.desired_example_name "title" .Params.title)}}