instructure / canvas-lms

The open LMS by Instructure, Inc.
https://github.com/instructure/canvas-lms/wiki
GNU Affero General Public License v3.0
5.4k stars 2.42k forks source link

message popover cannot be closed after sending a message since commit c638a66949f34ba7d438964687047985a20d5ada #2365

Closed breiter closed 2 weeks ago

breiter commented 2 weeks ago

Summary:

https://github.com/instructure/canvas-lms/commit/c638a66949f34ba7d438964687047985a20d5ada fixes a jquery issue where the attachment button did not work and logged javascript exceptions in issue #2343.

The commit resolves the attachment problem but introduces a problem:

Prior to sending a message you can close the message popover by clicking "cancel" or "x". After a message is sent the popover does not close and cannot be closed by the user. The "cancel" and "x" do not work. Also the attachment icon does not work at this point.

Steps to reproduce:

  1. Compose a message
  2. Send a message (message sent briefly appears)
  3. Message compose remains visually unchanged and does not go away on its own. Additionally, the "cancel", "x", and attachment buttons no longer work.

Expected behavior:

Message window should close or alternatively be reset to compose a new message. If the message window does not close on its own the "x", "cancel", and attachment buttons should work.

Actual behavior:

Compose window remains after sending and retains state. User can send the message over and over but cannot close the window.

Additional notes:

This broken UX results in users generating many duplicate messages.

omarpr commented 2 weeks ago

Any particular reason on why you are using the legacy inbox when we are now using the new react_inbox?

breiter commented 2 weeks ago

I don't think there is a strong reason other than the site admin not having enabled it. Will that setting resolve the problem?

breiter commented 2 weeks ago

Enabling "Update Inbox page to use react/instui" does not solve this problem.

omarpr commented 2 weeks ago

That's odd. It should solve the issue since the new React Inbox doesn't depend on jQuery. We would recommend to do that since we also have plans on removing soon the code for legacy inbox which we now consider dead code.

If after this you think the functionality you are accessing is visible even when using the new feature flag, can you expand the repro steps so we can take a closer look?

breiter commented 2 weeks ago

Inbox.

image

Click compose message.

image

Click send.

image

X, cancel, and attachment buttons now do not work. Compose window remains with the same content.

image

I discovered that I can click on the compose button in the "layer" behind the compose message window.

image

This resets the window and makes the X and cancel buttons work.

image

Nothing interesting is logged to the console.

changing the inbox react settings doesn't change the behavior.

image

The browser does not matter. These screenshots are Safari 17.5. The behavior is the same in Chrome 126.0.6478.57 and Firefox 127.0.

omarpr commented 2 weeks ago

Seems like you are having a server caching issue when changing the feature flag. That is indeed legacy inbox and we are removing that code very soon so it is a good time to figure out why your feature flag change isn't being recognized and fix it.

I have heard that you can try restarting your server after you change the feature flag to see if that helps with the caching issue.

breiter commented 2 weeks ago

Yes. It is a caching issue and the React version does work once the cache expires. This screenshot was after sending and the message window closed.

image
omarpr commented 2 weeks ago

Awesome. Seems like we are good to close this issue! :)