From 4adea67eb3bfca997d9665db72dae77a9f409265 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 15 Jul 2019 18:12:45 +0200 Subject: [PATCH 1/7] focus the composer in the body keydown handler if not other shortcuts apply this allows the user to start typing a message even if the composer is not focused. --- src/components/structures/LoggedInView.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index cd752fc2ce..146af704c0 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -333,6 +333,22 @@ const LoggedInView = React.createClass({ if (handled) { ev.stopPropagation(); ev.preventDefault(); + } else { + const targetTag = ev.target.tagName; + const focusedOnInputControl = targetTag === "INPUT" || + targetTag === "TEXTAREA" || + targetTag === "SELECT" || + ev.target.getAttribute("contenteditable"); + // we don't check for buttons that have focus here, + // because they should be using AccessibleButton which + // calls stopPropagation on space or enter keydown, so + // that keydown event would never get here. + if (!focusedOnInputControl) { + dis.dispatch({action: 'focus_composer'}, true); + ev.stopPropagation(); + // we should *not* preventDefault() here as + // that would prevent typing in the now-focussed composer + } } }, From 57abbc42730073365528179f1ea8ad3ceb270367 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 15 Jul 2019 18:21:10 +0200 Subject: [PATCH 2/7] remove unnessary manual focussing of composer now that composer is focused automatically when no other shortcuts apply, remove the manual focusing we have in place where it's not needed --- src/ContentMessages.js | 3 --- src/components/structures/MatrixChat.js | 9 --------- src/components/structures/RoomStatusBar.js | 2 -- src/components/views/elements/MessageEditor.js | 3 --- src/cryptodevices.js | 5 ----- 5 files changed, 22 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index 2d58622db8..9317e0b746 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -498,9 +498,6 @@ export default class ContentMessages { this.inprogress.push(upload); dis.dispatch({action: 'upload_started'}); - // Focus the composer view - dis.dispatch({action: 'focus_composer'}); - let error; function onProgress(ev) { diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index db5b7d034b..ad3c9ba3fa 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -268,8 +268,6 @@ export default React.createClass({ componentDidMount: function() { this.dispatcherRef = dis.register(this.onAction); - this.focusComposer = false; - // this can technically be done anywhere but doing this here keeps all // the routing url path logic together. if (this.onAliasClick) { @@ -362,10 +360,6 @@ export default React.createClass({ const durationMs = this.stopPageChangeTimer(); Analytics.trackPageChange(durationMs); } - if (this.focusComposer) { - dis.dispatch({action: 'focus_composer'}); - this.focusComposer = false; - } }, startPageChangeTimer() { @@ -793,8 +787,6 @@ export default React.createClass({ // that has been passed out-of-band (eg. // room name and avatar from an invite email) _viewRoom: function(roomInfo) { - this.focusComposer = true; - const newState = { view: VIEWS.LOGGED_IN, currentRoomId: roomInfo.room_id || null, @@ -1368,7 +1360,6 @@ export default React.createClass({ self.firstSyncComplete = true; self.firstSyncPromise.resolve(); - dis.dispatch({action: 'focus_composer'}); self.setState({ ready: true, showNotifierToolbar: Notifier.shouldShowToolbar(), diff --git a/src/components/structures/RoomStatusBar.js b/src/components/structures/RoomStatusBar.js index 7ef080e235..bf756b291b 100644 --- a/src/components/structures/RoomStatusBar.js +++ b/src/components/structures/RoomStatusBar.js @@ -135,12 +135,10 @@ module.exports = React.createClass({ _onResendAllClick: function() { Resend.resendUnsentEvents(this.props.room); - dis.dispatch({action: 'focus_composer'}); }, _onCancelAllClick: function() { Resend.cancelUnsentEvents(this.props.room); - dis.dispatch({action: 'focus_composer'}); }, _onShowDevicesClick: function() { diff --git a/src/components/views/elements/MessageEditor.js b/src/components/views/elements/MessageEditor.js index f51348ce04..86e79a3410 100644 --- a/src/components/views/elements/MessageEditor.js +++ b/src/components/views/elements/MessageEditor.js @@ -222,7 +222,6 @@ export default class MessageEditor extends React.Component { dis.dispatch({action: 'edit_event', event: nextEvent}); } else { dis.dispatch({action: 'edit_event', event: null}); - dis.dispatch({action: 'focus_composer'}); } event.preventDefault(); } @@ -230,7 +229,6 @@ export default class MessageEditor extends React.Component { _cancelEdit = () => { dis.dispatch({action: "edit_event", event: null}); - dis.dispatch({action: 'focus_composer'}); } _hasModifications(newContent) { @@ -257,7 +255,6 @@ export default class MessageEditor extends React.Component { this.context.matrixClient.sendMessage(roomId, editContent); dis.dispatch({action: "edit_event", event: null}); - dis.dispatch({action: 'focus_composer'}); } _cancelPreviousPendingEdit() { diff --git a/src/cryptodevices.js b/src/cryptodevices.js index 246fae3d73..b9a1fe869e 100644 --- a/src/cryptodevices.js +++ b/src/cryptodevices.js @@ -65,10 +65,6 @@ export async function getUnknownDevicesForRoom(matrixClient, room) { return unknownDevices; } -function focusComposer() { - dis.dispatch({action: 'focus_composer'}); -} - /** * Show the UnknownDeviceDialog for a given room. The dialog will inform the user * that messages they sent to this room have not been sent due to unknown devices @@ -90,7 +86,6 @@ export function showUnknownDeviceDialogForMessages(matrixClient, room) { sendAnywayLabel: _t("Send anyway"), sendLabel: _t("Send"), onSend: onSendClicked, - onFinished: focusComposer, }, 'mx_Dialog_unknownDevice'); }); } From 5c1c1f64b6623c1891f00da3438885f15745161c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 15 Jul 2019 18:25:22 +0200 Subject: [PATCH 3/7] fix lint --- src/cryptodevices.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cryptodevices.js b/src/cryptodevices.js index b9a1fe869e..54c96e3976 100644 --- a/src/cryptodevices.js +++ b/src/cryptodevices.js @@ -16,7 +16,6 @@ limitations under the License. import Resend from './Resend'; import sdk from './index'; -import dis from './dispatcher'; import Modal from './Modal'; import { _t } from './languageHandler'; From 10a74696b3c0340181f2da5c93901454fc20ec41 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jul 2019 16:50:05 +0200 Subject: [PATCH 4/7] hack around React having its own bubbling phase --- src/components/structures/LoggedInView.js | 42 +++++++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index 146af704c0..16618bb973 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -106,7 +106,7 @@ const LoggedInView = React.createClass({ CallMediaHandler.loadDevices(); - document.addEventListener('keydown', this._onKeyDown); + document.addEventListener('keydown', this._onNativeKeyDown, false); this._sessionStore = sessionStore; this._sessionStoreToken = this._sessionStore.addListener( @@ -136,7 +136,7 @@ const LoggedInView = React.createClass({ }, componentWillUnmount: function() { - document.removeEventListener('keydown', this._onKeyDown); + document.removeEventListener('keydown', this._onNativeKeyDown, false); this._matrixClient.removeListener("accountData", this.onAccountData); this._matrixClient.removeListener("sync", this.onSync); this._matrixClient.removeListener("RoomState.events", this.onRoomStateEvents); @@ -272,6 +272,42 @@ const LoggedInView = React.createClass({ }); }, + /* + SOME HACKERY BELOW: + React optimizes event handlers, by always attaching only 1 handler to the document for a given type. + It then internally determines the order in which React event handlers should be called, + emulating the capture and bubbling phases the DOM also has. + + But, as the native handler for React is always attached on the document, + it will always run last for bubbling (first for capturing) handlers, + and thus React basically has its own event phases, and will always run + after (before for capturing) any native other event handlers (as they tend to be attached last). + + So ideally one wouldn't mix React and native event handlers to have bubbling working as expected, + but we do need a native event handler here on the document, + to get keydown events when there is no focused element (target=body). + + We also do need bubbling here to give child components a chance to call `stopPropagation()`, + for keydown events it can handle itself, and shouldn't be redirected to the composer. + + So we listen with React on this component to get any events on focused elements, and get bubbling working as expected. + We also listen with a native listener on the document to get keydown events when no element is focused. + Bubbling is irrelevant here as the target is the body element. + */ + _onReactKeyDown: function(ev) { + // events caught while bubbling up on the root element + // of this component, so something must be focused. + this._onKeyDown(ev); + }, + + _onNativeKeyDown: function(ev) { + // only pass this if there is no focused element. + // if there is, _onKeyDown will be called by the + // react keydown handler that respects the react bubbling order. + if (ev.target === document.body) { + this._onKeyDown(ev); + } + }, _onKeyDown: function(ev) { /* @@ -560,7 +596,7 @@ const LoggedInView = React.createClass({ } return ( -
+
{ topBar }
From 4bde0c08ad5c3aaa95630e5110b6c8672bda94a7 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jul 2019 16:53:12 +0200 Subject: [PATCH 5/7] make sure we don't break any A or BUTTON keyboard handling --- src/components/structures/LoggedInView.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index 16618bb973..e3a99a42ca 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -374,12 +374,11 @@ const LoggedInView = React.createClass({ const focusedOnInputControl = targetTag === "INPUT" || targetTag === "TEXTAREA" || targetTag === "SELECT" || - ev.target.getAttribute("contenteditable"); - // we don't check for buttons that have focus here, - // because they should be using AccessibleButton which - // calls stopPropagation on space or enter keydown, so - // that keydown event would never get here. - if (!focusedOnInputControl) { + !!ev.target.getAttribute("contenteditable"); + const isClickShortcut = ev.target !== document.body && + (ev.key === "Space" || ev.key === "Enter"); + + if (!focusedOnInputControl && !isClickShortcut) { dis.dispatch({action: 'focus_composer'}, true); ev.stopPropagation(); // we should *not* preventDefault() here as From 50c88279a04b0b7076b5be770d29f0f650c6ed44 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jul 2019 17:05:56 +0200 Subject: [PATCH 6/7] remove non-existing keypress handler --- src/components/views/elements/Dropdown.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/Dropdown.js b/src/components/views/elements/Dropdown.js index d7569a0435..c99ae4f69d 100644 --- a/src/components/views/elements/Dropdown.js +++ b/src/components/views/elements/Dropdown.js @@ -48,7 +48,7 @@ class MenuOption extends React.Component { }); return
{ this.props.children } From 03f577bea9fa8bed0f038c9138e19c85a568a2a9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 17 Jul 2019 17:23:19 +0200 Subject: [PATCH 7/7] use keydown here, as its a div element --- src/components/views/elements/PowerSelector.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/elements/PowerSelector.js b/src/components/views/elements/PowerSelector.js index 9b886e7cb3..4089c4dd86 100644 --- a/src/components/views/elements/PowerSelector.js +++ b/src/components/views/elements/PowerSelector.js @@ -113,7 +113,7 @@ module.exports = React.createClass({ this.props.onChange(parseInt(this.state.customValue), this.props.powerLevelKey); }, - onCustomKeyPress: function(event) { + onCustomKeyDown: function(event) { if (event.key === "Enter") { event.preventDefault(); event.stopPropagation(); @@ -133,7 +133,7 @@ module.exports = React.createClass({ picker = ( ); } else {