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

First step of matrix-wysiwyg integration #9374

Merged
merged 22 commits into from Oct 11, 2022

Conversation

florianduros
Copy link
Contributor

@florianduros florianduros commented Oct 7, 2022

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Notes: Add the first step to integrate the matrix wysiwyg composer

matrix wysiwyg composer

  • In this first step, the WysiwygComposer can only send messages. in furthers PRs, the remaining features will be added
  • This new composer is hided behind the feature_wysiwyg_composer lab flag
  • The wysiwyg will replace the SendMessageComposer, so a part of its code is duplicated.
  • This code contains a lot of todos for reply, thread, emoji which will be supported in the next wysiwyg PRs.
  • The CSS and Style are also temporary

Here's what your changelog entry will look like:

✨ Features

  • Add the first step to integrate the matrix wysiwyg composer (#9374). Contributed by @florianduros.

@florianduros florianduros added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Oct 10, 2022
Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

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

Looks good to me, added some non-blocking comments 👍
Pretty excited about a revamp of the composer

Comment on lines +182 to +187
if (SettingsStore.getValue("scrollToBottomOnMessageSent")) {
dis.dispatch({
action: "scroll_to_bottom",
timelineRenderingType: roomContext.timelineRenderingType,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced this should live in here. This looks like a side effect that should be done when catching the message_sent action.

Also do not think this should block this pull request from landing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I copied the behaviour of the SendMessageComposer here. My goal here is to integrate the base of the matrix-wysiwyg and not change the current sendMessage behaviour.

} });

const memoizedSendMessage = useCallback(() => {
sendMessage(content, { mxClient, roomContext, ...props });
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about making sendMessage a prop instead? That way we can keep the business logic separate and reusable, and this component easy to test.
I also find the renderProps binding of sendMessage a bit weird

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the parent already have the html content thanks to the onChange, the sendMessage is not needed here and the logic can be in the parent. I will add a component in the next PRs between the WysiwygComposer and the MessageComposer. I'll avoid to add extra logic in the MessageComposer because it's already big enough.

I'm also not convinced by the memoizedSendMessage in the children but I'm searching a way to nicely do the focus and clear. In the old composer a ref is used by I clearly don't like this solution.

const wrapper = wrapAndRender({ room });

SettingsStore.setValue("feature_wysiwyg_composer", null, SettingLevel.DEVICE, false);
expect(wrapper.find(WysiwygComposer)).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it still rendered after changing the setting to false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change of settings doesn't trigger a rerender. Since SettingsStore is a singleton, I set the old value here

@florianduros florianduros merged commit b336e18 into develop Oct 11, 2022
@florianduros florianduros deleted the feat/matrix-wysisyg-integration branch October 11, 2022 09:08
su-ex added a commit to SchildiChat/matrix-react-sdk that referenced this pull request Oct 29, 2022
* Include a file-safe room name and ISO date in chat exports ([\matrix-org#9440](matrix-org#9440)). Fixes element-hq/element-web#21812 and element-hq/element-web#19724.
* Room call banner ([\matrix-org#9378](matrix-org#9378)). Fixes element-hq/element-web#23453. Contributed by @toger5.
* Device manager - spinners while devices are signing out ([\matrix-org#9433](matrix-org#9433)). Fixes element-hq/element-web#15865.
* Device manager - silence call ringers when local notifications are silenced ([\matrix-org#9420](matrix-org#9420)).
* Pass the current language to Element Call ([\matrix-org#9427](matrix-org#9427)).
* Hide screen-sharing button in Element Call on desktop ([\matrix-org#9423](matrix-org#9423)).
* Add reply support to WysiwygComposer ([\matrix-org#9422](matrix-org#9422)). Contributed by @florianduros.
* Disconnect other connected devices (of the same user) when joining an Element call ([\matrix-org#9379](matrix-org#9379)).
* Device manager - device tile main click target ([\matrix-org#9409](matrix-org#9409)).
* Add formatting buttons to the rich text editor ([\matrix-org#9410](matrix-org#9410)). Contributed by @florianduros.
* Device manager - current session context menu ([\matrix-org#9386](matrix-org#9386)).
* Remove piwik config fallback for privacy policy URL ([\matrix-org#9390](matrix-org#9390)).
* Add the first step to integrate the matrix wysiwyg composer ([\matrix-org#9374](matrix-org#9374)). Contributed by @florianduros.
* Device manager - UA parsing tweaks ([\matrix-org#9382](matrix-org#9382)).
* Device manager - remove client information events when disabling setting ([\matrix-org#9384](matrix-org#9384)).
* Add Element Call participant limit ([\matrix-org#9358](matrix-org#9358)).
* Add Element Call room settings ([\matrix-org#9347](matrix-org#9347)).
* Device manager - render extended device information ([\matrix-org#9360](matrix-org#9360)).
* New group call experience: Room header and PiP designs ([\matrix-org#9351](matrix-org#9351)).
* Pass language to Jitsi Widget ([\matrix-org#9346](matrix-org#9346)). Contributed by @Fox32.
* Add notifications and toasts for Element Call calls ([\matrix-org#9337](matrix-org#9337)).
* Device manager - device type icon ([\matrix-org#9355](matrix-org#9355)).
* Delete the remainder of groups ([\matrix-org#9357](matrix-org#9357)). Fixes element-hq/element-web#22770.
* Device manager - display client information in device details ([\matrix-org#9315](matrix-org#9315)).
* Send Content-Type: application/json header for integration manager /register API ([\matrix-org#9490](matrix-org#9490)). Fixes element-hq/element-web#23580.
* Device manager - put client/browser device metadata in correct section ([\matrix-org#9447](matrix-org#9447)).
* update the room unread notification counter when the server changes the value without any related read receipt ([\matrix-org#9438](matrix-org#9438)).
* Don't show call banners in video rooms ([\matrix-org#9441](matrix-org#9441)).
* Prevent useContextMenu isOpen from being true if the button ref goes away ([\matrix-org#9418](matrix-org#9418)). Fixes matrix-org/element-web-rageshakes#15637.
* Automatically focus the WYSIWYG composer when you enter a room ([\matrix-org#9412](matrix-org#9412)).
* Improve the tooltips on the call lobby join button ([\matrix-org#9428](matrix-org#9428)).
* Pass the homeserver's base URL to Element Call ([\matrix-org#9429](matrix-org#9429)). Fixes element-hq/element-web#23301.
* Better accommodate long room names in call toasts ([\matrix-org#9426](matrix-org#9426)).
* Hide virtual widgets from the room info panel ([\matrix-org#9424](matrix-org#9424)). Fixes element-hq/element-web#23494.
* Inhibit clicking on sender avatar in threads list ([\matrix-org#9417](matrix-org#9417)). Fixes element-hq/element-web#23482.
* Correct the dir parameter of MSC3715 ([\matrix-org#9391](matrix-org#9391)). Contributed by @dhenneke.
* Use a more correct subset of users in `/remakeolm` developer command ([\matrix-org#9402](matrix-org#9402)).
* use correct default for notification silencing ([\matrix-org#9388](matrix-org#9388)). Fixes element-hq/element-web#23456.
* Device manager - eagerly create `m.local_notification_settings` events ([\matrix-org#9353](matrix-org#9353)).
* Close incoming Element call toast when viewing the call lobby ([\matrix-org#9375](matrix-org#9375)).
* Always allow enabling sending read receipts ([\matrix-org#9367](matrix-org#9367)). Fixes element-hq/element-web#23433.
* Fixes (element-hq/element-web/issues/22609) where the white theme is not applied when `white -> dark -> white` sequence is done. ([\matrix-org#9320](matrix-org#9320)). Contributed by @florianduros.
* Fix applying programmatically set height for "top" room layout ([\matrix-org#9339](matrix-org#9339)). Contributed by @Fox32.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants