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.

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

Modal Widget close actions are not interpreted anymore #21498

Closed
dhenneke opened this issue Mar 21, 2022 · 0 comments · Fixed by matrix-org/matrix-react-sdk#8123
Closed

Modal Widget close actions are not interpreted anymore #21498

dhenneke opened this issue Mar 21, 2022 · 0 comments · Fixed by matrix-org/matrix-react-sdk#8123
Labels
A-Widgets O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect X-Regression

Comments

@dhenneke
Copy link
Contributor

dhenneke commented Mar 21, 2022

Steps to reproduce

  1. Open a new modal from a widget with matrixWidgetApi.openModalWidget(...).
  2. Await the closing message with matrixWidgetApi.on('action:close_modal', ...).
  3. Close the widget with matrixWidgetApi.closeModalWidget(...).

Outcome

What did you expect?

The close message is received by the event handler.

What happened instead?

The event is ignored and element throws an error.

image


This seems to be introduced by matrix-org/matrix-react-sdk#8052.

In the code, the ModalWidgetStore#closeModalWidget() call forwards the optional! widgetRoomId to WidgetMessagingStore#getMessaging() that has a required! widgetRoomId argument. Finally, WidgetUtils#calcWidgetUid(...) returns user_<widget-id> instead of room_<room-id>_<widget-id>.

The solution seems to be that the StopGapWidget should forward the correct room id:

  private onOpenModal = async (ev: CustomEvent<IModalWidgetOpenRequest>) => {
      ev.preventDefault();
      if (ModalWidgetStore.instance.canOpenModalWidget()) {
-         ModalWidgetStore.instance.openModalWidget(ev.detail.data, this.mockWidget);
+         ModalWidgetStore.instance.openModalWidget(ev.detail.data, this.mockWidget, this.roomId);
          this.messaging.transport.reply(ev.detail, {}); // ack
      } else {
          this.messaging.transport.reply(ev.detail, {
              error: {
                  message: "Unable to open modal at this time",
              },
          });
      }
  };

The string vs string | undefined situation is still not optimal, but at least the correct id is generated in my tests.

Operating system

No response

Browser information

No response

URL for webapp

No response

Application version

No response

Homeserver

No response

Will you send logs?

No

@germain-gg germain-gg added S-Minor Impairs non-critical functionality or suitable workarounds exist A-Widgets O-Uncommon Most users are unlikely to come across this or unexpected workflow X-Regression labels Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Widgets O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect X-Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants