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
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
bfb1638
Add wysisyg composer (can only send message, enable behind a labs flag)
florianduros Oct 5, 2022
1d820cf
Remove console, unused variables...
florianduros Oct 5, 2022
a50329f
Focus and clear content after sending a message
florianduros Oct 7, 2022
4938aa8
Remove unused code
florianduros Oct 7, 2022
21677e6
Remove unused MatrixClient
florianduros Oct 7, 2022
c7f9125
Move mxClient to object in sendMessage
florianduros Oct 7, 2022
bcc53fc
Add style for WysiwygComposer
florianduros Oct 7, 2022
67aab08
Update yarn.lock
florianduros Oct 7, 2022
667e8ef
Add tests to message.ts
florianduros Oct 10, 2022
3080d14
Remove unused localstorage hook
florianduros Oct 10, 2022
200af78
Use published matrix-wysisyg
florianduros Oct 10, 2022
a0377f0
Update wysiwig flag description
florianduros Oct 10, 2022
101fd62
Add WysiwygComposer test
florianduros Oct 10, 2022
77005e2
Update wysiwyg version
florianduros Oct 10, 2022
5bdac78
Merge remote-tracking branch 'origin/develop' into feat/matrix-wysisy…
florianduros Oct 10, 2022
ec1140e
Fix type errors
florianduros Oct 10, 2022
6c71581
Add test in MessageComposer
florianduros Oct 10, 2022
70f5779
Add more todo in message.ts
florianduros Oct 10, 2022
f8ec4ec
Disable wysiwyg at the end of the test
florianduros Oct 10, 2022
7ad39ba
Merge branch 'develop' into feat/matrix-wysisyg-integration
florianduros Oct 10, 2022
0dd9aba
Fix aria-disabled
florianduros Oct 11, 2022
203f75f
Merge branch 'develop' into feat/matrix-wysisyg-integration
florianduros Oct 11, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -57,6 +57,7 @@
"dependencies": {
"@babel/runtime": "^7.12.5",
"@matrix-org/analytics-events": "^0.2.0",
"@matrix-org/matrix-wysiwyg": "^0.0.2",
"@matrix-org/react-sdk-module-api": "^0.0.3",
"@sentry/browser": "^6.11.0",
"@sentry/tracing": "^6.11.0",
Expand Down
1 change: 1 addition & 0 deletions res/css/_components.pcss
Expand Up @@ -295,6 +295,7 @@
@import "./views/rooms/_TopUnreadMessagesBar.pcss";
@import "./views/rooms/_VoiceRecordComposerTile.pcss";
@import "./views/rooms/_WhoIsTypingTile.pcss";
@import "./views/rooms/wysiwyg_composer/_WysiwygComposer.pcss";
@import "./views/settings/_AvatarSetting.pcss";
@import "./views/settings/_CrossSigningPanel.pcss";
@import "./views/settings/_CryptographyPanel.pcss";
Expand Down
53 changes: 53 additions & 0 deletions res/css/views/rooms/wysiwyg_composer/_WysiwygComposer.pcss
@@ -0,0 +1,53 @@
/*
Copyright 2022 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

.mx_WysiwygComposer {
flex: 1;
display: flex;
flex-direction: column;
font-size: $font-14px;
/* fixed line height to prevent emoji from being taller than text */
line-height: $font-18px;
justify-content: center;
margin-right: 6px;
/* don't grow wider than available space */
min-width: 0;

.mx_WysiwygComposer_container {
flex: 1;
display: flex;
flex-direction: column;
/* min-height at this level so the mx_BasicMessageComposer_input */
/* still stays vertically centered when less than 55px. */
/* We also set this to ensure the voice message recording widget */
/* doesn't cause a jump. */
min-height: 55px;

.mx_WysiwygComposer_content {
border: 1px solid;
border-radius: 20px;
padding: 8px 10px;
/* this will center the contenteditable */
/* in it's parent vertically */
/* while keeping the autocomplete at the top */
/* of the composer. The parent needs to be a flex container for this to work. */
margin: auto 0;
/* max-height at this level so autocomplete doesn't get scrolled too */
max-height: 140px;
overflow-y: auto;
}
}
}
54 changes: 40 additions & 14 deletions src/components/views/rooms/MessageComposer.tsx
Expand Up @@ -58,6 +58,7 @@ import {
startNewVoiceBroadcastRecording,
VoiceBroadcastRecordingsStore,
} from '../../../voice-broadcast';
import { WysiwygComposer } from './wysiwyg_composer/WysiwygComposer';

let instanceCount = 0;

Expand Down Expand Up @@ -105,6 +106,7 @@ export default class MessageComposer extends React.Component<IProps, IState> {
private voiceRecordingButton = createRef<VoiceRecordComposerTile>();
private ref: React.RefObject<HTMLDivElement> = createRef();
private instanceId: number;
private composerSendMessage?: () => void;

private _voiceRecording: Optional<VoiceMessageRecording>;

Expand Down Expand Up @@ -313,6 +315,7 @@ export default class MessageComposer extends React.Component<IProps, IState> {
}

this.messageComposerInput.current?.sendMessage();
this.composerSendMessage?.();
};

private onChange = (model: EditorModel) => {
Expand All @@ -321,6 +324,12 @@ export default class MessageComposer extends React.Component<IProps, IState> {
});
};

private onWysiwygChange = (content: string) => {
this.setState({
isComposerEmpty: content?.length === 0,
});
};

private onVoiceStoreUpdate = () => {
this.updateRecordingState();
};
Expand Down Expand Up @@ -394,20 +403,37 @@ export default class MessageComposer extends React.Component<IProps, IState> {

const canSendMessages = this.context.canSendMessages && !this.context.tombstone;
if (canSendMessages) {
controls.push(
<SendMessageComposer
ref={this.messageComposerInput}
key="controls_input"
room={this.props.room}
placeholder={this.renderPlaceholderText()}
permalinkCreator={this.props.permalinkCreator}
relation={this.props.relation}
replyToEvent={this.props.replyToEvent}
onChange={this.onChange}
disabled={this.state.haveRecording}
toggleStickerPickerOpen={this.toggleStickerPickerOpen}
/>,
);
const isWysiwygComposerEnabled = SettingsStore.getValue("feature_wysiwyg_composer");

if (isWysiwygComposerEnabled) {
controls.push(
<WysiwygComposer key="controls_input"
disabled={this.state.haveRecording}
onChange={this.onWysiwygChange}
permalinkCreator={this.props.permalinkCreator}
relation={this.props.relation}
replyToEvent={this.props.replyToEvent}>
{ (sendMessage) => {
this.composerSendMessage = sendMessage;
} }
</WysiwygComposer>,
);
} else {
controls.push(
<SendMessageComposer
ref={this.messageComposerInput}
key="controls_input"
room={this.props.room}
placeholder={this.renderPlaceholderText()}
permalinkCreator={this.props.permalinkCreator}
relation={this.props.relation}
replyToEvent={this.props.replyToEvent}
onChange={this.onChange}
disabled={this.state.haveRecording}
toggleStickerPickerOpen={this.toggleStickerPickerOpen}
/>,
);
}

controls.push(<VoiceRecordComposerTile
key="controls_voice_record"
Expand Down
71 changes: 71 additions & 0 deletions src/components/views/rooms/wysiwyg_composer/WysiwygComposer.tsx
@@ -0,0 +1,71 @@
/*
Copyright 2022 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { useCallback, useState } from 'react';
import { useWysiwyg } from "@matrix-org/matrix-wysiwyg";
import { IEventRelation, MatrixEvent } from 'matrix-js-sdk/src/models/event';

import { useRoomContext } from '../../../../contexts/RoomContext';
import { sendMessage } from './message';
import { RoomPermalinkCreator } from '../../../../utils/permalinks/Permalinks';
import { useMatrixClientContext } from '../../../../contexts/MatrixClientContext';

interface WysiwygProps {
disabled?: boolean;
onChange: (content: string) => void;
relation?: IEventRelation;
replyToEvent?: MatrixEvent;
permalinkCreator: RoomPermalinkCreator;
includeReplyLegacyFallback?: boolean;
children?: (sendMessage: () => void) => void;
}

export function WysiwygComposer(
{ disabled = false, onChange, children, ...props }: WysiwygProps,
) {
const roomContext = useRoomContext();
const mxClient = useMatrixClientContext();

const [content, setContent] = useState<string>();
const { ref, isWysiwygReady, wysiwyg } = useWysiwyg({ onChange: (_content) => {
setContent(_content);
onChange(_content);
} });

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.

wysiwyg.clear();
ref.current?.focus();
}, [content, mxClient, roomContext, wysiwyg, props, ref]);

return (
<div className="mx_WysiwygComposer">
<div className="mx_WysiwygComposer_container">
<div className="mx_WysiwygComposer_content"
ref={ref}
contentEditable={!disabled && isWysiwygReady}
role="textbox"
aria-multiline="true"
aria-autocomplete="list"
aria-haspopup="listbox"
dir="auto"
aria-disabled={disabled || !isWysiwygReady}
/>
</div>
{ children?.(memoizedSendMessage) }
</div>
);
}