From cbf951569eb7a7164473deec1badb1ad7572a18d Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 25 Feb 2022 00:51:01 +0100 Subject: [PATCH] Fix reply button on media modal not giving focus to compose form (#17626) * Avoid compose form and modal management fighting for focus * Fix reply button on media modal footer not giving focus to compose form --- app/javascript/mastodon/actions/modal.js | 3 +- .../mastodon/components/modal_root.js | 5 +++- .../compose/components/compose_form.js | 9 ++++-- .../picture_in_picture/components/footer.js | 2 +- .../features/ui/components/modal_root.js | 9 +++--- .../features/ui/containers/modal_container.js | 11 +++---- app/javascript/mastodon/reducers/modal.js | 30 +++++++++++++++---- 7 files changed, 50 insertions(+), 19 deletions(-) diff --git a/app/javascript/mastodon/actions/modal.js b/app/javascript/mastodon/actions/modal.js index 3d0299db58..3e576fab8e 100644 --- a/app/javascript/mastodon/actions/modal.js +++ b/app/javascript/mastodon/actions/modal.js @@ -9,9 +9,10 @@ export function openModal(type, props) { }; }; -export function closeModal(type) { +export function closeModal(type, options = { ignoreFocus: false }) { return { type: MODAL_CLOSE, modalType: type, + ignoreFocus: options.ignoreFocus, }; }; diff --git a/app/javascript/mastodon/components/modal_root.js b/app/javascript/mastodon/components/modal_root.js index 755c46fd6b..b894aeaf98 100644 --- a/app/javascript/mastodon/components/modal_root.js +++ b/app/javascript/mastodon/components/modal_root.js @@ -18,6 +18,7 @@ export default class ModalRoot extends React.PureComponent { g: PropTypes.number, b: PropTypes.number, }), + ignoreFocus: PropTypes.bool, }; activeElement = this.props.children ? document.activeElement : null; @@ -72,7 +73,9 @@ export default class ModalRoot extends React.PureComponent { // immediately selectable, we have to wait for observers to run, as // described in https://github.com/WICG/inert#performance-and-gotchas Promise.resolve().then(() => { - this.activeElement.focus({ preventScroll: true }); + if (!this.props.ignoreFocus) { + this.activeElement.focus({ preventScroll: true }); + } this.activeElement = null; }).catch(console.error); diff --git a/app/javascript/mastodon/features/compose/components/compose_form.js b/app/javascript/mastodon/features/compose/components/compose_form.js index d75145a09e..d7635da404 100644 --- a/app/javascript/mastodon/features/compose/components/compose_form.js +++ b/app/javascript/mastodon/features/compose/components/compose_form.js @@ -163,8 +163,13 @@ class ComposeForm extends ImmutablePureComponent { selectionStart = selectionEnd; } - this.autosuggestTextarea.textarea.setSelectionRange(selectionStart, selectionEnd); - this.autosuggestTextarea.textarea.focus(); + // Because of the wicg-inert polyfill, the activeElement may not be + // immediately selectable, we have to wait for observers to run, as + // described in https://github.com/WICG/inert#performance-and-gotchas + Promise.resolve().then(() => { + this.autosuggestTextarea.textarea.setSelectionRange(selectionStart, selectionEnd); + this.autosuggestTextarea.textarea.focus(); + }).catch(console.error); } else if(prevProps.isSubmitting && !this.props.isSubmitting) { this.autosuggestTextarea.textarea.focus(); } else if (this.props.spoiler !== prevProps.spoiler) { diff --git a/app/javascript/mastodon/features/picture_in_picture/components/footer.js b/app/javascript/mastodon/features/picture_in_picture/components/footer.js index 0de562ee1d..690a77531c 100644 --- a/app/javascript/mastodon/features/picture_in_picture/components/footer.js +++ b/app/javascript/mastodon/features/picture_in_picture/components/footer.js @@ -60,7 +60,7 @@ class Footer extends ImmutablePureComponent { const { router } = this.context; if (onClose) { - onClose(); + onClose(true); } dispatch(replyCompose(status, router.history)); diff --git a/app/javascript/mastodon/features/ui/components/modal_root.js b/app/javascript/mastodon/features/ui/components/modal_root.js index 7b14fe5caa..3fc235849d 100644 --- a/app/javascript/mastodon/features/ui/components/modal_root.js +++ b/app/javascript/mastodon/features/ui/components/modal_root.js @@ -45,6 +45,7 @@ export default class ModalRoot extends React.PureComponent { type: PropTypes.string, props: PropTypes.object, onClose: PropTypes.func.isRequired, + ignoreFocus: PropTypes.bool, }; state = { @@ -79,7 +80,7 @@ export default class ModalRoot extends React.PureComponent { return ; } - handleClose = () => { + handleClose = (ignoreFocus = false) => { const { onClose } = this.props; let message = null; try { @@ -89,7 +90,7 @@ export default class ModalRoot extends React.PureComponent { // isn't set. // This would be much smoother with react-intl 3+ and `forwardRef`. } - onClose(message); + onClose(message, ignoreFocus); } setModalRef = (c) => { @@ -97,12 +98,12 @@ export default class ModalRoot extends React.PureComponent { } render () { - const { type, props } = this.props; + const { type, props, ignoreFocus } = this.props; const { backgroundColor } = this.state; const visible = !!type; return ( - + {visible && ( {(SpecificComponent) => } diff --git a/app/javascript/mastodon/features/ui/containers/modal_container.js b/app/javascript/mastodon/features/ui/containers/modal_container.js index 34fec82066..35be262221 100644 --- a/app/javascript/mastodon/features/ui/containers/modal_container.js +++ b/app/javascript/mastodon/features/ui/containers/modal_container.js @@ -3,22 +3,23 @@ import { openModal, closeModal } from '../../../actions/modal'; import ModalRoot from '../components/modal_root'; const mapStateToProps = state => ({ - type: state.getIn(['modal', 0, 'modalType'], null), - props: state.getIn(['modal', 0, 'modalProps'], {}), + ignoreFocus: state.getIn(['modal', 'ignoreFocus']), + type: state.getIn(['modal', 'stack', 0, 'modalType'], null), + props: state.getIn(['modal', 'stack', 0, 'modalProps'], {}), }); const mapDispatchToProps = dispatch => ({ - onClose (confirmationMessage) { + onClose (confirmationMessage, ignoreFocus = false) { if (confirmationMessage) { dispatch( openModal('CONFIRM', { message: confirmationMessage.message, confirm: confirmationMessage.confirm, - onConfirm: () => dispatch(closeModal()), + onConfirm: () => dispatch(closeModal(undefined, { ignoreFocus })), }), ); } else { - dispatch(closeModal()); + dispatch(closeModal(undefined, { ignoreFocus })); } }, }); diff --git a/app/javascript/mastodon/reducers/modal.js b/app/javascript/mastodon/reducers/modal.js index 41161a2060..3eab07d9d4 100644 --- a/app/javascript/mastodon/reducers/modal.js +++ b/app/javascript/mastodon/reducers/modal.js @@ -3,16 +3,36 @@ import { TIMELINE_DELETE } from '../actions/timelines'; import { COMPOSE_UPLOAD_CHANGE_SUCCESS } from '../actions/compose'; import { Stack as ImmutableStack, Map as ImmutableMap } from 'immutable'; -export default function modal(state = ImmutableStack(), action) { +const initialState = ImmutableMap({ + ignoreFocus: false, + stack: ImmutableStack(), +}); + +const popModal = (state, { modalType, ignoreFocus }) => { + if (modalType === undefined || modalType === state.getIn(['stack', 0, 'modalType'])) { + return state.set('ignoreFocus', !!ignoreFocus).update('stack', stack => stack.shift()); + } else { + return state; + } +}; + +const pushModal = (state, modalType, modalProps) => { + return state.withMutations(map => { + map.set('ignoreFocus', false); + map.update('stack', stack => stack.unshift(ImmutableMap({ modalType, modalProps }))); + }); +}; + +export default function modal(state = initialState, action) { switch(action.type) { case MODAL_OPEN: - return state.unshift(ImmutableMap({ modalType: action.modalType, modalProps: action.modalProps })); + return pushModal(state, action.modalType, action.modalProps); case MODAL_CLOSE: - return (action.modalType === undefined || action.modalType === state.getIn([0, 'modalType'])) ? state.shift() : state; + return popModal(state, action); case COMPOSE_UPLOAD_CHANGE_SUCCESS: - return state.getIn([0, 'modalType']) === 'FOCAL_POINT' ? state.shift() : state; + return popModal(state, { modalType: 'FOCAL_POINT', ignoreFocus: false }); case TIMELINE_DELETE: - return state.filterNot((modal) => modal.get('modalProps').statusId === action.id); + return state.update('stack', stack => stack.filterNot((modal) => modal.get('modalProps').statusId === action.id)); default: return state; }