From cb6f445b90f1bf13e6aaa8ca8165642487801146 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 25 May 2023 19:14:51 +0200 Subject: [PATCH] Greatly simplify history management code (#2230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2220 This drops the ability to shift+click on “Back” to get back to a pinned column, but that was inconsistent, broken, and undocumented. This also brings us slightly closer to upstream. --- .../glitch/components/column_back_button.jsx | 18 +++++------- .../components/column_back_button_slim.jsx | 18 +++++------- .../glitch/components/column_header.jsx | 29 ++++++++----------- .../flavours/glitch/components/permalink.jsx | 4 +-- .../flavours/glitch/components/status.jsx | 12 ++------ .../glitch/components/status_action_bar.jsx | 5 ++-- .../components/moved_note.jsx | 4 +-- .../components/conversation.jsx | 4 +-- .../status/components/detailed_status.jsx | 8 ++--- .../flavours/glitch/features/status/index.jsx | 4 +-- .../features/ui/components/boost_modal.jsx | 4 +-- .../ui/components/favourite_modal.jsx | 4 +-- 12 files changed, 41 insertions(+), 73 deletions(-) diff --git a/app/javascript/flavours/glitch/components/column_back_button.jsx b/app/javascript/flavours/glitch/components/column_back_button.jsx index 9a3afe8b3..003d37671 100644 --- a/app/javascript/flavours/glitch/components/column_back_button.jsx +++ b/app/javascript/flavours/glitch/components/column_back_button.jsx @@ -14,17 +14,15 @@ export default class ColumnBackButton extends React.PureComponent { multiColumn: PropTypes.bool, }; - handleClick = (event) => { - // if history is exhausted, or we would leave mastodon, just go to root. - if (window.history.state) { - const state = this.context.router.history.location.state; - if (event.shiftKey && state && state.mastodonBackSteps) { - this.context.router.history.go(-state.mastodonBackSteps); - } else { - this.context.router.history.goBack(); - } + handleClick = () => { + const { router } = this.context; + + // Check if there is a previous page in the app to go back to per https://stackoverflow.com/a/70532858/9703201 + // When upgrading to V6, check `location.key !== 'default'` instead per https://github.com/remix-run/history/blob/main/docs/api-reference.md#location + if (router.route.location.key) { + router.history.goBack(); } else { - this.context.router.history.push('/'); + router.history.push('/'); } }; diff --git a/app/javascript/flavours/glitch/components/column_back_button_slim.jsx b/app/javascript/flavours/glitch/components/column_back_button_slim.jsx index b27ead1c7..6fb800640 100644 --- a/app/javascript/flavours/glitch/components/column_back_button_slim.jsx +++ b/app/javascript/flavours/glitch/components/column_back_button_slim.jsx @@ -9,17 +9,15 @@ export default class ColumnBackButtonSlim extends React.PureComponent { router: PropTypes.object, }; - handleClick = (event) => { - // if history is exhausted, or we would leave mastodon, just go to root. - if (window.history.state) { - const state = this.context.router.history.location.state; - if (event.shiftKey && state && state.mastodonBackSteps) { - this.context.router.history.go(-state.mastodonBackSteps); - } else { - this.context.router.history.goBack(); - } + handleClick = () => { + const { router } = this.context; + + // Check if there is a previous page in the app to go back to per https://stackoverflow.com/a/70532858/9703201 + // When upgrading to V6, check `location.key !== 'default'` instead per https://github.com/remix-run/history/blob/main/docs/api-reference.md#location + if (router.route.location.key) { + router.history.goBack(); } else { - this.context.router.history.push('/'); + router.history.push('/'); } }; diff --git a/app/javascript/flavours/glitch/components/column_header.jsx b/app/javascript/flavours/glitch/components/column_header.jsx index bd2660595..c4b6e9247 100644 --- a/app/javascript/flavours/glitch/components/column_header.jsx +++ b/app/javascript/flavours/glitch/components/column_header.jsx @@ -42,20 +42,6 @@ class ColumnHeader extends React.PureComponent { animating: false, }; - historyBack = (skip) => { - // if history is exhausted, or we would leave mastodon, just go to root. - if (window.history.state) { - const state = this.context.router.history.location.state; - if (skip && state && state.mastodonBackSteps) { - this.context.router.history.go(-state.mastodonBackSteps); - } else { - this.context.router.history.goBack(); - } - } else { - this.context.router.history.push('/'); - } - }; - handleToggleClick = (e) => { e.stopPropagation(); this.setState({ collapsed: !this.state.collapsed, animating: true }); @@ -73,8 +59,16 @@ class ColumnHeader extends React.PureComponent { this.props.onMove(1); }; - handleBackClick = (event) => { - this.historyBack(event.shiftKey); + handleBackClick = () => { + const { router } = this.context; + + // Check if there is a previous page in the app to go back to per https://stackoverflow.com/a/70532858/9703201 + // When upgrading to V6, check `location.key !== 'default'` instead per https://github.com/remix-run/history/blob/main/docs/api-reference.md#location + if (router.route.location.key) { + router.history.goBack(); + } else { + router.history.push('/'); + } }; handleTransitionEnd = () => { @@ -83,8 +77,9 @@ class ColumnHeader extends React.PureComponent { handlePin = () => { if (!this.props.pinned) { - this.historyBack(); + this.context.router.history.replace('/'); } + this.props.onPin(); }; diff --git a/app/javascript/flavours/glitch/components/permalink.jsx b/app/javascript/flavours/glitch/components/permalink.jsx index b09b17eeb..1c5843183 100644 --- a/app/javascript/flavours/glitch/components/permalink.jsx +++ b/app/javascript/flavours/glitch/components/permalink.jsx @@ -24,9 +24,7 @@ export default class Permalink extends React.PureComponent { if (this.context.router) { e.preventDefault(); - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(this.props.to, state); + this.context.router.history.push(this.props.to); } } }; diff --git a/app/javascript/flavours/glitch/components/status.jsx b/app/javascript/flavours/glitch/components/status.jsx index 25eac0d45..78deb5f2a 100644 --- a/app/javascript/flavours/glitch/components/status.jsx +++ b/app/javascript/flavours/glitch/components/status.jsx @@ -368,9 +368,7 @@ class Status extends ImmutablePureComponent { status.getIn(['reblog', 'id'], status.get('id')) }`; } - let state = { ...router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - router.history.push(destination, state); + router.history.push(destination); } e.preventDefault(); } @@ -441,16 +439,12 @@ class Status extends ImmutablePureComponent { }; handleHotkeyOpen = () => { - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; const status = this.props.status; - this.context.router.history.push(`/@${status.getIn(['account', 'acct'])}/${status.get('id')}`, state); + this.context.router.history.push(`/@${status.getIn(['account', 'acct'])}/${status.get('id')}`); }; handleHotkeyOpenProfile = () => { - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`, state); + this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`); }; handleHotkeyMoveUp = e => { diff --git a/app/javascript/flavours/glitch/components/status_action_bar.jsx b/app/javascript/flavours/glitch/components/status_action_bar.jsx index 5b87cd439..20f6dd800 100644 --- a/app/javascript/flavours/glitch/components/status_action_bar.jsx +++ b/app/javascript/flavours/glitch/components/status_action_bar.jsx @@ -163,10 +163,9 @@ class StatusActionBar extends ImmutablePureComponent { handleOpen = () => { let state = { ...this.context.router.history.location.state }; if (state.mastodonModalKey) { - this.context.router.history.replace(`/@${this.props.status.getIn(['account', 'acct'])}/${this.props.status.get('id')}`, { mastodonBackSteps: (state.mastodonBackSteps || 0) + 1 }); + this.context.router.history.replace(`/@${this.props.status.getIn(['account', 'acct'])}/${this.props.status.get('id')}`); } else { - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}/${this.props.status.get('id')}`, state); + this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}/${this.props.status.get('id')}`); } }; diff --git a/app/javascript/flavours/glitch/features/account_timeline/components/moved_note.jsx b/app/javascript/flavours/glitch/features/account_timeline/components/moved_note.jsx index 8f9693f64..908b70c5f 100644 --- a/app/javascript/flavours/glitch/features/account_timeline/components/moved_note.jsx +++ b/app/javascript/flavours/glitch/features/account_timeline/components/moved_note.jsx @@ -21,9 +21,7 @@ export default class MovedNote extends ImmutablePureComponent { handleAccountClick = e => { if (e.button === 0) { e.preventDefault(); - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(`/@${this.props.to.get('acct')}`, state); + this.context.router.history.push(`/@${this.props.to.get('acct')}`); } e.stopPropagation(); diff --git a/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.jsx b/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.jsx index 485f2d022..18bc6e575 100644 --- a/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.jsx +++ b/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.jsx @@ -59,9 +59,7 @@ class Conversation extends ImmutablePureComponent { } destination = `/statuses/${lastStatus.get('id')}`; } - let state = { ...router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - router.history.push(destination, state); + router.history.push(destination); e.preventDefault(); } }; diff --git a/app/javascript/flavours/glitch/features/status/components/detailed_status.jsx b/app/javascript/flavours/glitch/features/status/components/detailed_status.jsx index 26f40c3e0..992dd677d 100644 --- a/app/javascript/flavours/glitch/features/status/components/detailed_status.jsx +++ b/app/javascript/flavours/glitch/features/status/components/detailed_status.jsx @@ -55,9 +55,7 @@ class DetailedStatus extends ImmutablePureComponent { handleAccountClick = (e) => { if (e.button === 0 && !(e.ctrlKey || e.altKey || e.metaKey) && this.context.router) { e.preventDefault(); - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`, state); + this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`); } e.stopPropagation(); @@ -66,9 +64,7 @@ class DetailedStatus extends ImmutablePureComponent { parseClick = (e, destination) => { if (e.button === 0 && !(e.ctrlKey || e.altKey || e.metaKey) && this.context.router) { e.preventDefault(); - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(destination, state); + this.context.router.history.push(destination); } e.stopPropagation(); diff --git a/app/javascript/flavours/glitch/features/status/index.jsx b/app/javascript/flavours/glitch/features/status/index.jsx index 46e49177e..09c1aa992 100644 --- a/app/javascript/flavours/glitch/features/status/index.jsx +++ b/app/javascript/flavours/glitch/features/status/index.jsx @@ -502,9 +502,7 @@ class Status extends ImmutablePureComponent { }; handleHotkeyOpenProfile = () => { - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`, state); + this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`); }; handleMoveUp = id => { diff --git a/app/javascript/flavours/glitch/features/ui/components/boost_modal.jsx b/app/javascript/flavours/glitch/features/ui/components/boost_modal.jsx index 939e9765e..31ae4f68f 100644 --- a/app/javascript/flavours/glitch/features/ui/components/boost_modal.jsx +++ b/app/javascript/flavours/glitch/features/ui/components/boost_modal.jsx @@ -62,9 +62,7 @@ class BoostModal extends ImmutablePureComponent { if (e.button === 0) { e.preventDefault(); this.props.onClose(); - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`, state); + this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`); } }; diff --git a/app/javascript/flavours/glitch/features/ui/components/favourite_modal.jsx b/app/javascript/flavours/glitch/features/ui/components/favourite_modal.jsx index 03e93cd27..4090b6500 100644 --- a/app/javascript/flavours/glitch/features/ui/components/favourite_modal.jsx +++ b/app/javascript/flavours/glitch/features/ui/components/favourite_modal.jsx @@ -43,9 +43,7 @@ class FavouriteModal extends ImmutablePureComponent { if (e.button === 0) { e.preventDefault(); this.props.onClose(); - let state = { ...this.context.router.history.location.state }; - state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1; - this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`, state); + this.context.router.history.push(`/@${this.props.status.getIn(['account', 'acct'])}`); } };