Merge pull request #1566 from ClearlyClaire/glitch-soc/feature/modal-stack

Fix boost/fav confirmation modals closing media modal

app/javascript/flavours/glitch/components/modal_root.js

@@ -76,10 +76,13 @@ export default class ModalRoot extends React.PureComponent {
         this.activeElement = null;
       }).catch(console.error);
 
-      this.handleModalClose();
+      this._handleModalClose();
     }
     if (this.props.children && !prevProps.children) {
-      this.handleModalOpen();
+      this._handleModalOpen();
+    }
+    if (this.props.children) {
+      this._ensureHistoryBuffer();
     }
   }
 
@@ -88,22 +91,29 @@ export default class ModalRoot extends React.PureComponent {
     window.removeEventListener('keydown', this.handleKeyDown);
   }
 
-  handleModalClose () {
+  _handleModalOpen () {
+    this._modalHistoryKey = Date.now();
+    this.unlistenHistory = this.history.listen((_, action) => {
+      if (action === 'POP') {
+        this.props.onClose();
+      }
+    });
+  }
+
+  _handleModalClose () {
     this.unlistenHistory();
 
-    const state = this.history.location.state;
-    if (state && state.mastodonModalOpen) {
+    const { state } = this.history.location;
+    if (state && state.mastodonModalKey === this._modalHistoryKey) {
       this.history.goBack();
     }
   }
 
-  handleModalOpen () {
-    const history = this.history;
-    const state   = {...history.location.state, mastodonModalOpen: true};
-    history.push(history.location.pathname, state);
-    this.unlistenHistory = history.listen(() => {
-      this.props.onClose();
-    });
+  _ensureHistoryBuffer () {
+    const { pathname, state } = this.history.location;
+    if (!state || state.mastodonModalKey !== this._modalHistoryKey) {
+      this.history.push(pathname, { ...state, mastodonModalKey: this._modalHistoryKey });
+    }
   }
 
   getSiblings = () => {

app/javascript/flavours/glitch/components/scrollable_list.js

@@ -1,5 +1,5 @@
 import React, { PureComponent } from 'react';
-import { ScrollContainer } from 'react-router-scroll-4';
+import ScrollContainer from 'flavours/glitch/containers/scroll_container';
 import PropTypes from 'prop-types';
 import IntersectionObserverArticleContainer from 'flavours/glitch/containers/intersection_observer_article_container';
 import LoadMore from './load_more';
@@ -34,7 +34,6 @@ class ScrollableList extends PureComponent {
     onScrollToTop: PropTypes.func,
     onScroll: PropTypes.func,
     trackScroll: PropTypes.bool,
-    shouldUpdateScroll: PropTypes.func,
     isLoading: PropTypes.bool,
     showLoading: PropTypes.bool,
     hasMore: PropTypes.bool,
@@ -264,11 +263,6 @@ class ScrollableList extends PureComponent {
     this.props.onLoadMore();
   }
 
-  defaultShouldUpdateScroll = (prevRouterProps, { location }) => {
-    if ((((prevRouterProps || {}).location || {}).state || {}).mastodonModalOpen) return false;
-    return !(location.state && location.state.mastodonModalOpen);
-  }
-
   handleLoadPending = e => {
     e.preventDefault();
     this.props.onLoadPending();
@@ -282,7 +276,7 @@ class ScrollableList extends PureComponent {
   }
 
   render () {
-    const { children, scrollKey, trackScroll, shouldUpdateScroll, showLoading, isLoading, hasMore, numPending, prepend, alwaysPrepend, append, emptyMessage, onLoadMore } = this.props;
+    const { children, scrollKey, trackScroll, showLoading, isLoading, hasMore, numPending, prepend, alwaysPrepend, append, emptyMessage, onLoadMore } = this.props;
     const { fullscreen } = this.state;
     const childrenCount = React.Children.count(children);
 
@@ -348,7 +342,7 @@ class ScrollableList extends PureComponent {
 
     if (trackScroll) {
       return (
-        <ScrollContainer scrollKey={scrollKey} shouldUpdateScroll={shouldUpdateScroll || this.defaultShouldUpdateScroll}>
+        <ScrollContainer scrollKey={scrollKey}>
           {scrollableArea}
         </ScrollContainer>
       );

app/javascript/flavours/glitch/components/status_action_bar.js

@@ -147,7 +147,7 @@ class StatusActionBar extends ImmutablePureComponent {
 
   handleOpen = () => {
     let state = {...this.context.router.history.location.state};
-    if (state.mastodonModalOpen) {
+    if (state.mastodonModalKey) {
       this.context.router.history.replace(`/statuses/${this.props.status.get('id')}`, { mastodonBackSteps: (state.mastodonBackSteps || 0) + 1 });
     } else {
       state.mastodonBackSteps = (state.mastodonBackSteps || 0) + 1;

app/javascript/flavours/glitch/components/status_list.js

@@ -18,7 +18,6 @@ export default class StatusList extends ImmutablePureComponent {
     onScrollToTop: PropTypes.func,
     onScroll: PropTypes.func,
     trackScroll: PropTypes.bool,
-    shouldUpdateScroll: PropTypes.func,
     isLoading: PropTypes.bool,
     isPartial: PropTypes.bool,
     hasMore: PropTypes.bool,

app/javascript/flavours/glitch/containers/mastodon.js

@@ -41,7 +41,7 @@ export default class Mastodon extends React.PureComponent {
   }
 
   shouldUpdateScroll (_, { location }) {
-    return !(location.state && location.state.mastodonModalOpen);
+    return !(location.state?.mastodonModalKey);
   }
 
   render () {

app/javascript/flavours/glitch/containers/scroll_container.js

@@ -0,0 +1,18 @@
+import { ScrollContainer as OriginalScrollContainer } from 'react-router-scroll-4';
+
+// ScrollContainer is used to automatically scroll to the top when pushing a
+// new history state and remembering the scroll position when going back.
+// There are a few things we need to do differently, though.
+const defaultShouldUpdateScroll = (prevRouterProps, { location }) => {
+  // If the change is caused by opening a modal, do not scroll to top
+  return !(location.state?.mastodonModalKey && location.state?.mastodonModalKey !== prevRouterProps?.location?.state?.mastodonModalKey);
+};
+
+export default
+class ScrollContainer extends OriginalScrollContainer {
+
+  static defaultProps = {
+    shouldUpdateScroll: defaultShouldUpdateScroll,
+  };
+
+}

app/javascript/flavours/glitch/features/account_gallery/index.js

@@ -11,7 +11,7 @@ import ImmutablePureComponent from 'react-immutable-pure-component';
 import { getAccountGallery } from 'flavours/glitch/selectors';
 import MediaItem from './components/media_item';
 import HeaderContainer from 'flavours/glitch/features/account_timeline/containers/header_container';
-import { ScrollContainer } from 'react-router-scroll-4';
+import ScrollContainer from 'flavours/glitch/containers/scroll_container';
 import LoadMore from 'flavours/glitch/components/load_more';
 import MissingIndicator from 'flavours/glitch/components/missing_indicator';
 import { openModal } from 'flavours/glitch/actions/modal';
@@ -104,11 +104,6 @@ class AccountGallery extends ImmutablePureComponent {
     this.handleScrollToBottom();
   }
 
-  shouldUpdateScroll = (prevRouterProps, { location }) => {
-    if ((((prevRouterProps || {}).location || {}).state || {}).mastodonModalOpen) return false;
-    return !(location.state && location.state.mastodonModalOpen);
-  }
-
   setColumnRef = c => {
     this.column = c;
   }
@@ -165,7 +160,7 @@ class AccountGallery extends ImmutablePureComponent {
       <Column ref={this.setColumnRef}>
         <ProfileColumnHeader onClick={this.handleHeaderClick} multiColumn={multiColumn} />
 
-        <ScrollContainer scrollKey='account_gallery' shouldUpdateScroll={this.shouldUpdateScroll}>
+        <ScrollContainer scrollKey='account_gallery'>
           <div className='scrollable scrollable--flex' onScroll={this.handleScroll}>
             <HeaderContainer accountId={this.props.params.accountId} />
 

app/javascript/flavours/glitch/features/directory/index.js

@@ -12,7 +12,7 @@ import AccountCard from './components/account_card';
 import RadioButton from 'flavours/glitch/components/radio_button';
 import classNames from 'classnames';
 import LoadMore from 'flavours/glitch/components/load_more';
-import { ScrollContainer } from 'react-router-scroll-4';
+import ScrollContainer from 'flavours/glitch/containers/scroll_container';
 
 const messages = defineMessages({
   title: { id: 'column.directory', defaultMessage: 'Browse profiles' },
@@ -40,7 +40,6 @@ class Directory extends React.PureComponent {
     isLoading: PropTypes.bool,
     accountIds: ImmutablePropTypes.list.isRequired,
     dispatch: PropTypes.func.isRequired,
-    shouldUpdateScroll: PropTypes.func,
     columnId: PropTypes.string,
     intl: PropTypes.object.isRequired,
     multiColumn: PropTypes.bool,
@@ -125,7 +124,7 @@ class Directory extends React.PureComponent {
   }
 
   render () {
-    const { isLoading, accountIds, intl, columnId, multiColumn, domain, shouldUpdateScroll } = this.props;
+    const { isLoading, accountIds, intl, columnId, multiColumn, domain } = this.props;
     const { order, local }  = this.getParams(this.props, this.state);
     const pinned = !!columnId;
 
@@ -163,7 +162,7 @@ class Directory extends React.PureComponent {
           multiColumn={multiColumn}
         />
 
-        {multiColumn && !pinned ? <ScrollContainer scrollKey='directory' shouldUpdateScroll={shouldUpdateScroll}>{scrollableArea}</ScrollContainer> : scrollableArea}
+        {multiColumn && !pinned ? <ScrollContainer scrollKey='directory'>{scrollableArea}</ScrollContainer> : scrollableArea}
       </Column>
     );
   }

app/javascript/flavours/glitch/features/notifications/index.js

@@ -99,7 +99,6 @@ class Notifications extends React.PureComponent {
     notifications: ImmutablePropTypes.list.isRequired,
     showFilterBar: PropTypes.bool.isRequired,
     dispatch: PropTypes.func.isRequired,
-    shouldUpdateScroll: PropTypes.func,
     intl: PropTypes.object.isRequired,
     isLoading: PropTypes.bool,
     isUnread: PropTypes.bool,
@@ -220,7 +219,7 @@ class Notifications extends React.PureComponent {
   }
 
   render () {
-    const { intl, notifications, shouldUpdateScroll, isLoading, isUnread, columnId, multiColumn, hasMore, numPending, showFilterBar, lastReadId, canMarkAsRead, needsNotificationPermission } = this.props;
+    const { intl, notifications, isLoading, isUnread, columnId, multiColumn, hasMore, numPending, showFilterBar, lastReadId, canMarkAsRead, needsNotificationPermission } = this.props;
     const { notifCleaning, notifCleaningActive } = this.props;
     const { animatingNCD } = this.state;
     const pinned = !!columnId;
@@ -273,7 +272,6 @@ class Notifications extends React.PureComponent {
         onLoadPending={this.handleLoadPending}
         onScrollToTop={this.handleScrollToTop}
         onScroll={this.handleScroll}
-        shouldUpdateScroll={shouldUpdateScroll}
         bindToDocument={!multiColumn}
       >
         {scrollableContent}

app/javascript/flavours/glitch/features/status/index.js

@@ -32,7 +32,7 @@ import { initBlockModal } from 'flavours/glitch/actions/blocks';
 import { initReport } from 'flavours/glitch/actions/reports';
 import { initBoostModal } from 'flavours/glitch/actions/boosts';
 import { makeGetStatus } from 'flavours/glitch/selectors';
-import { ScrollContainer } from 'react-router-scroll-4';
+import ScrollContainer from 'flavours/glitch/containers/scroll_container';
 import ColumnBackButton from 'flavours/glitch/components/column_back_button';
 import ColumnHeader from '../../components/column_header';
 import StatusContainer from 'flavours/glitch/containers/status_container';
@@ -507,11 +507,6 @@ class Status extends ImmutablePureComponent {
     this.setState({ fullscreen: isFullscreen() });
   }
 
-  shouldUpdateScroll = (prevRouterProps, { location }) => {
-    if ((((prevRouterProps || {}).location || {}).state || {}).mastodonModalOpen) return false;
-    return !(location.state && location.state.mastodonModalOpen);
-  }
-
   render () {
     let ancestors, descendants;
     const { setExpansion } = this;
@@ -562,7 +557,7 @@ class Status extends ImmutablePureComponent {
           )}
         />
 
-        <ScrollContainer scrollKey='thread' shouldUpdateScroll={this.shouldUpdateScroll}>
+        <ScrollContainer scrollKey='thread'>
           <div className={classNames('scrollable', 'detailed-status__wrapper', { fullscreen })} ref={this.setRef}>
             {ancestors}
 

app/javascript/flavours/glitch/features/ui/components/modal_root.js

@@ -59,12 +59,8 @@ export default class ModalRoot extends React.PureComponent {
     backgroundColor: null,
   };
 
-  getSnapshotBeforeUpdate () {
-    return { visible: !!this.props.type };
-  }
-
-  componentDidUpdate (prevProps, prevState, { visible }) {
-    if (visible) {
+  componentDidUpdate () {
+    if (!!this.props.type) {
       document.body.classList.add('with-modals--active');
       document.documentElement.style.marginRight = `${getScrollbarWidth()}px`;
     } else {

app/javascript/flavours/glitch/features/ui/containers/modal_container.js

@@ -3,8 +3,8 @@ import { closeModal } from 'flavours/glitch/actions/modal';
 import ModalRoot from '../components/modal_root';
 
 const mapStateToProps = state => ({
-  type: state.get('modal').modalType,
-  props: state.get('modal').modalProps,
+  type: state.getIn(['modal', 0, 'modalType'], null),
+  props: state.getIn(['modal', 0, 'modalProps'], {}),
 });
 
 const mapDispatchToProps = dispatch => ({

app/javascript/flavours/glitch/features/ui/index.js

@@ -212,7 +212,7 @@ class SwitchingColumnsArea extends React.PureComponent {
 
           <WrappedRoute path='/start' component={FollowRecommendations} content={children} />
           <WrappedRoute path='/search' component={Search} content={children} />
-          <WrappedRoute path='/directory' component={Directory} content={children} componentParams={{ shouldUpdateScroll: this.shouldUpdateScroll }} />
+          <WrappedRoute path='/directory' component={Directory} content={children} />
 
           <WrappedRoute path='/statuses/new' component={Compose} content={children} />
           <WrappedRoute path='/statuses/:statusId' exact component={Status} content={children} />

app/javascript/flavours/glitch/reducers/modal.js

@@ -1,19 +1,15 @@
 import { MODAL_OPEN, MODAL_CLOSE } from 'flavours/glitch/actions/modal';
 import { TIMELINE_DELETE } from 'flavours/glitch/actions/timelines';
+import { Stack as ImmutableStack, Map as ImmutableMap } from 'immutable';
 
-const initialState = {
-  modalType: null,
-  modalProps: {},
-};
-
-export default function modal(state = initialState, action) {
+export default function modal(state = ImmutableStack(), action) {
   switch(action.type) {
   case MODAL_OPEN:
-    return { modalType: action.modalType, modalProps: action.modalProps };
+    return state.unshift(ImmutableMap({ modalType: action.modalType, modalProps: action.modalProps }));
   case MODAL_CLOSE:
-    return (action.modalType === undefined || action.modalType === state.modalType) ? initialState : state;
+    return (action.modalType === undefined || action.modalType === state.getIn([0, 'modalType'])) ? state.shift() : state;
   case TIMELINE_DELETE:
-    return (state.modalProps.statusId === action.id) ? initialState : state;
+    return state.filterNot((modal) => modal.get('modalProps').statusId === action.id);
   default:
     return state;
   }