Fix inline spoiler text in modals

This requires “connecting” `<StatusContent>`, which isn’t “great” but
is basically necessary at this point. On the plus side, this makes the
code a bit DRYer.

Due to the way modals work, updating status contents doesn’t actually
trigger a React rerender. This is actually fine, because we (have to)
update status contents live anyway, but it does require ensuring that
appropriate event listeners get attached so that the spoiler can be
toggled again.

To keep listeners from being added multiple times (which wouldn’t have
a negative effect but could be a performance leak), elements with
listeners are now tracked in a `WeakSet`.

diff --git a/app/javascript/flavours/glitch/components/status.js b/app/javascript/flavours/glitch/components/status.js
index 1bb0d72a445c9f0352ce51b06420bdecdb12dcbc..147a426936958dc3265d8e4fb85f5ecc7b22c9cd 100644 (file)
--- a/app/javascript/flavours/glitch/components/status.js
+++ b/app/javascript/flavours/glitch/components/status.js
@@ -4,7 +4,7 @@ import PropTypes from 'prop-types';
 import StatusPrepend from './status_prepend';
 import StatusHeader from './status_header';
 import StatusIcons from './status_icons';
-import StatusContent from './status_content';
+import StatusContentContainer from 'flavours/glitch/containers/status_content_container';
 import StatusActionBar from './status_action_bar';
 import AttachmentList from './attachment_list';
 import Card from '../features/status/components/card';
@@ -775,14 +775,13 @@ class Status extends ImmutablePureComponent {
               settings={settings.get('status_icons')}
             />
           </header>
-          <StatusContent
+          <StatusContentContainer
             status={status}
             media={contentMedia}
             extraMedia={extraMedia}
             mediaIcons={contentMediaIcons}
             expanded={isExpanded}
             onExpandedToggle={this.handleExpandedToggle}
-            onToggleSpoilerText={onToggleSpoilerText}
             parseClick={parseClick}
             disabled={!router}
             tagLinks={settings.get('tag_misleading_links')}

diff --git a/app/javascript/flavours/glitch/components/status_content.js b/app/javascript/flavours/glitch/components/status_content.js
index 8aa03ad715b5046eacd0110ebba18a528630dcad..8b8e43503324fcaa1f76d0f3c6cb9335029c735a 100644 (file)
--- a/app/javascript/flavours/glitch/components/status_content.js
+++ b/app/javascript/flavours/glitch/components/status_content.js
@@ -1,7 +1,8 @@
 import React from 'react';
 import ImmutablePropTypes from 'react-immutable-proptypes';
+import ImmutablePureComponent from 'react-immutable-pure-component';
 import PropTypes from 'prop-types';
-import { injectIntl, FormattedMessage } from 'react-intl';
+import { FormattedMessage } from 'react-intl';
 import Permalink from './permalink';
 import classnames from 'classnames';
 import Icon from 'flavours/glitch/components/icon';
@@ -62,8 +63,9 @@ const isLinkMisleading = (link) => {
   return !(textMatchesTarget(text, origin, host) || textMatchesTarget(text.toLowerCase(), origin, host));
 };
 
-@injectIntl
-export default class StatusContent extends React.PureComponent {
+const listening = new WeakSet();
+
+export default class StatusContent extends ImmutablePureComponent {
 
   static propTypes = {
     status: ImmutablePropTypes.map.isRequired,
@@ -91,6 +93,29 @@ export default class StatusContent extends React.PureComponent {
     hidden: true,
   };
 
+  /** Define listeners for inline spoiler text elements. */
+  _defineSpoilerTextListeners () {
+    const node = this.contentsNode;
+    const { onToggleSpoilerText } = this.props;
+    if (node && onToggleSpoilerText) {
+      [...node.querySelectorAll('.spoilertext')].forEach((elt) => {
+        if (listening.has(elt)) return;
+        elt.querySelector('button').addEventListener(
+          'click',
+          (e) => {
+            // Ordinarily, calling the `onSpoilerTextClict(elt)` method
+            // will trigger a rerender, but it may not in the case of
+            // modals. This means we may need to do a manual redefining
+            // of spoiler text listeners.
+            this.onSpoilerTextClick(elt, e);
+            this._defineSpoilerTextListeners();
+          },
+        );
+        listening.add(elt);
+      });
+    }
+  }
+
   _updateStatusLinks () {
     const node = this.contentsNode;
     const { tagLinks, rewriteMentions } = this.props;
@@ -99,13 +124,6 @@ export default class StatusContent extends React.PureComponent {
       return;
     }
 
-    [...node.querySelectorAll('.spoilertext')].forEach((elt) => {
-      elt.querySelector('button').addEventListener(
-        'click',
-        this.onSpoilerTextClick.bind(this, elt),
-      );
-    });
-
     const links = node.querySelectorAll('a');
 
     for (var i = 0; i < links.length; ++i) {
@@ -193,10 +211,12 @@ export default class StatusContent extends React.PureComponent {
 
   componentDidMount () {
     const node = this.contentsNode;
-    if (node) {
+    const { onToggleSpoilerText } = this.props;
+    if (node && onToggleSpoilerText) {
       // Replace spoiler texts with their internationalized versions.
       [...node.querySelectorAll('.spoilertext')].forEach((elt) => {
-        this.props.onToggleSpoilerText(
+        if (node.querySelector('.spoilertext--button').title) return;
+        onToggleSpoilerText(
           this.props.status,
           node,
           elt,
@@ -208,10 +228,12 @@ export default class StatusContent extends React.PureComponent {
     // The `.onToggleSpoilerText()` method actually replaces the
     // `.spoilertext` elements, so we need to call this *after*.
     this._updateStatusLinks();
+    this._defineSpoilerTextListeners();
   }
 
   componentDidUpdate () {
     this._updateStatusLinks();
+    this._defineSpoilerTextListeners();
     if (this.props.onUpdate) this.props.onUpdate();
   }
 

diff --git a/app/javascript/flavours/glitch/containers/status_container.js b/app/javascript/flavours/glitch/containers/status_container.js
index ee2e5c644aafd27b81e22d6f0866d213b75e5380..c50b6be4a04e7535b39e9e050a7231880488aeab 100644 (file)
--- a/app/javascript/flavours/glitch/containers/status_container.js
+++ b/app/javascript/flavours/glitch/containers/status_container.js
@@ -24,7 +24,6 @@ import {
   hideStatus,
   revealStatus,
   editStatus,
-  modifyStatusBody,
 } from 'flavours/glitch/actions/statuses';
 import {
   initAddFilter,
@@ -43,7 +42,6 @@ import { showAlertForError } from '../actions/alerts';
 import AccountContainer from 'flavours/glitch/containers/account_container';
 import Spoilers from '../components/spoilers';
 import Icon from 'flavours/glitch/components/icon';
-import spoilertextify from 'flavours/glitch/utils/spoilertextify';
 
 const messages = defineMessages({
   deleteConfirm: { id: 'confirmations.delete.confirm', defaultMessage: 'Delete' },
@@ -234,26 +232,6 @@ const mapDispatchToProps = (dispatch, { intl, contextType }) => ({
     }
   },
 
-  onToggleSpoilerText (status, oldBody, spoilerElement, intl, open) {
-    spoilerElement.replaceWith(spoilertextify(
-      spoilerElement.getAttribute('data-spoilertext-content'),
-      {
-        emojos: status.get('emojis').reduce((obj, emoji) => {
-          obj[`:${emoji.get('shortcode')}:`] = emoji.toJS();
-          return obj;
-        }, {}),
-        intl,
-        open: open == null
-          ? !spoilerElement.classList.contains('open')
-          : !!open,
-      },
-    ));
-    dispatch(modifyStatusBody(
-      status.get('id'),
-      oldBody.innerHTML,
-    ));
-  },
-
   deployPictureInPicture (status, type, mediaProps) {
     dispatch((_, getState) => {
       if (getState().getIn(['local_settings', 'media', 'pop_in_player'])) {

diff --git a/app/javascript/flavours/glitch/containers/status_content_container.js b/app/javascript/flavours/glitch/containers/status_content_container.js
new file mode 100644 (file)
index 0000000..05541ca
--- /dev/null
+++ b/app/javascript/flavours/glitch/containers/status_content_container.js
@@ -0,0 +1,36 @@
+import { connect } from 'react-redux';
+import { injectIntl } from 'react-intl';
+import StatusContent from 'flavours/glitch/components/status_content';
+import { modifyStatusBody } from 'flavours/glitch/actions/statuses';
+import spoilertextify from 'flavours/glitch/utils/spoilertextify';
+
+const mapDispatchToProps = (dispatch, { intl, contextType }) => ({
+  /**
+   * Modifies the spoiler to be open or closed and then rewrites the
+   * HTML of the status to reflect that state.
+   *
+   * This will also save any other changes to the HTML, for example
+   * link rewriting.
+   */
+  onToggleSpoilerText (status, oldBody, spoilerElement, intl, open) {
+    spoilerElement.replaceWith(spoilertextify(
+      spoilerElement.getAttribute('data-spoilertext-content'),
+      {
+        emojos: status.get('emojis').reduce((obj, emoji) => {
+          obj[`:${emoji.get('shortcode')}:`] = emoji.toJS();
+          return obj;
+        }, {}),
+        intl,
+        open: open == null
+          ? !spoilerElement.classList.contains('open')
+          : !!open,
+      },
+    ));
+    dispatch(modifyStatusBody(
+      status.get('id'),
+      oldBody.innerHTML,
+    ));
+  },
+});
+
+export default injectIntl(connect(null, mapDispatchToProps)(StatusContent));
\ No newline at end of file

diff --git a/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.js b/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.js
index 7107c9db3bc0bf20f61ded1f6f8436637a415565..3db7e76510bed486c74b1a4a35a3062f40de8e53 100644 (file)
--- a/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.js
+++ b/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.js
@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import ImmutablePureComponent from 'react-immutable-pure-component';
-import StatusContent from 'flavours/glitch/components/status_content';
+import StatusContentContainer from 'flavours/glitch/containers/status_content_container';
 import AttachmentList from 'flavours/glitch/components/attachment_list';
 import { defineMessages, injectIntl, FormattedMessage } from 'react-intl';
 import DropdownMenuContainer from 'flavours/glitch/containers/dropdown_menu_container';
@@ -199,7 +199,7 @@ class Conversation extends ImmutablePureComponent {
               </div>
             </div>
 
-            <StatusContent
+            <StatusContentContainer
               status={lastStatus}
               parseClick={this.parseClick}
               expanded={isExpanded}

diff --git a/app/javascript/flavours/glitch/features/report/components/status_check_box.js b/app/javascript/flavours/glitch/features/report/components/status_check_box.js
index 76bf0eb851763201be8e432d07398d5912a6c0ee..71353e003c42651d2de2bd4f40a1a5240ea6e342 100644 (file)
--- a/app/javascript/flavours/glitch/features/report/components/status_check_box.js
+++ b/app/javascript/flavours/glitch/features/report/components/status_check_box.js
@@ -1,7 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import ImmutablePropTypes from 'react-immutable-proptypes';
-import StatusContent from 'flavours/glitch/components/status_content';
+import StatusContentContainer from 'flavours/glitch/containers/status_content_container';
 import Avatar from 'flavours/glitch/components/avatar';
 import DisplayName from 'flavours/glitch/components/display_name';
 import RelativeTimestamp from 'flavours/glitch/components/relative_timestamp';
@@ -39,7 +39,7 @@ export default class StatusCheckBox extends React.PureComponent {
           <div><DisplayName account={status.get('account')} /> · <RelativeTimestamp timestamp={status.get('created_at')} /></div>
         </div>
 
-        <StatusContent status={status} media={<MediaAttachments status={status} revealed={false} />} />
+        <StatusContentContainer status={status} media={<MediaAttachments status={status} revealed={false} />} />
       </div>
     );
 

diff --git a/app/javascript/flavours/glitch/features/status/components/detailed_status.js b/app/javascript/flavours/glitch/features/status/components/detailed_status.js
index 9bd4e0f245a2608ca309f3ed75fb5a7fdcaf3d47..566b953ff245cec837362ea1e9969e24e9e3e750 100644 (file)
--- a/app/javascript/flavours/glitch/features/status/components/detailed_status.js
+++ b/app/javascript/flavours/glitch/features/status/components/detailed_status.js
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import Avatar from 'flavours/glitch/components/avatar';
 import DisplayName from 'flavours/glitch/components/display_name';
-import StatusContent from 'flavours/glitch/components/status_content';
+import StatusContentContainer from 'flavours/glitch/containers/status_content_container';
 import MediaGallery from 'flavours/glitch/components/media_gallery';
 import AttachmentList from 'flavours/glitch/components/attachment_list';
 import { Link } from 'react-router-dom';
@@ -35,7 +35,6 @@ class DetailedStatus extends ImmutablePureComponent {
     onOpenMedia: PropTypes.func.isRequired,
     onOpenVideo: PropTypes.func.isRequired,
     onToggleHidden: PropTypes.func,
-    onToggleSpoilerText: PropTypes.func,
     expanded: PropTypes.bool,
     measureHeight: PropTypes.bool,
     onHeightChange: PropTypes.func,
@@ -116,7 +115,7 @@ class DetailedStatus extends ImmutablePureComponent {
 
   render () {
     const status = (this.props.status && this.props.status.get('reblog')) ? this.props.status.get('reblog') : this.props.status;
-    const { expanded, onToggleHidden, onToggleSpoilerText, settings, usingPiP, intl } = this.props;
+    const { expanded, onToggleHidden, settings, usingPiP, intl } = this.props;
     const outerStyle = { boxSizing: 'border-box' };
     const { compact } = this.props;
 
@@ -299,7 +298,7 @@ class DetailedStatus extends ImmutablePureComponent {
             <DisplayName account={status.get('account')} localDomain={this.props.domain} />
           </a>
 
-          <StatusContent
+          <StatusContentContainer
             status={status}
             media={contentMedia}
             extraMedia={extraMedia}
@@ -307,7 +306,6 @@ class DetailedStatus extends ImmutablePureComponent {
             expanded={expanded}
             collapsed={false}
             onExpandedToggle={onToggleHidden}
-            onToggleSpoilerText={onToggleSpoilerText}
             parseClick={this.parseClick}
             onUpdate={this.handleChildUpdate}
             tagLinks={settings.get('tag_misleading_links')}

diff --git a/app/javascript/flavours/glitch/features/status/index.js b/app/javascript/flavours/glitch/features/status/index.js
index 1ce30fcad08d436f4cb1ae992a6eaaf6e88bbfd9..dfc83279ba1a29be4c4fc8a1f36732371e5666d1 100644 (file)
--- a/app/javascript/flavours/glitch/features/status/index.js
+++ b/app/javascript/flavours/glitch/features/status/index.js
@@ -33,7 +33,6 @@ import {
   editStatus,
   hideStatus,
   revealStatus,
-  modifyStatusBody,
 } from 'flavours/glitch/actions/statuses';
 import { initMuteModal } from 'flavours/glitch/actions/mutes';
 import { initBlockModal } from 'flavours/glitch/actions/blocks';
@@ -53,7 +52,6 @@ import { attachFullscreenListener, detachFullscreenListener, isFullscreen } from
 import { autoUnfoldCW } from 'flavours/glitch/util/content_warning';
 import { textForScreenReader, defaultMediaVisibility } from 'flavours/glitch/components/status';
 import Icon from 'flavours/glitch/components/icon';
-import spoilertextify from 'flavours/glitch/utils/spoilertextify';
 
 const messages = defineMessages({
   deleteConfirm: { id: 'confirmations.delete.confirm', defaultMessage: 'Delete' },
@@ -238,26 +236,6 @@ class Status extends ImmutablePureComponent {
     }
   }
 
-  handleToggleSpoilerText = (status, oldBody, spoilerElement, intl, open) => {
-    spoilerElement.replaceWith(spoilertextify(
-      spoilerElement.getAttribute('data-spoilertext-content'),
-      {
-        emojos: status.get('emojis').reduce((obj, emoji) => {
-          obj[`:${emoji.get('shortcode')}:`] = emoji.toJS();
-          return obj;
-        }, {}),
-        intl,
-        open: open == null
-          ? !spoilerElement.classList.contains('open')
-          : !!open,
-      },
-    ));
-    this.props.dispatch(modifyStatusBody(
-      status.get('id'),
-      oldBody.innerHTML,
-    ));
-  }
-
   handleToggleMediaVisibility = () => {
     this.setState({ showMedia: !this.state.showMedia });
   }
@@ -623,7 +601,6 @@ class Status extends ImmutablePureComponent {
                   settings={settings}
                   onOpenVideo={this.handleOpenVideo}
                   onOpenMedia={this.handleOpenMedia}
-                  onToggleSpoilerText={this.handleToggleSpoilerText}
                   expanded={isExpanded}
                   onToggleHidden={this.handleToggleHidden}
                   domain={domain}

diff --git a/app/javascript/flavours/glitch/features/ui/components/actions_modal.js b/app/javascript/flavours/glitch/features/ui/components/actions_modal.js
index aae2e4426d1164f6d1dcc1197f53048f4ca66699..8173bcdc1fba0ecb6a6dadcda9b3651144970375 100644 (file)
--- a/app/javascript/flavours/glitch/features/ui/components/actions_modal.js
+++ b/app/javascript/flavours/glitch/features/ui/components/actions_modal.js
@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import ImmutablePureComponent from 'react-immutable-pure-component';
-import StatusContent from 'flavours/glitch/components/status_content';
+import StatusContentContainer from 'flavours/glitch/containers/status_content_container';
 import Avatar from 'flavours/glitch/components/avatar';
 import RelativeTimestamp from 'flavours/glitch/components/relative_timestamp';
 import DisplayName from 'flavours/glitch/components/display_name';
@@ -73,7 +73,7 @@ export default class ActionsModal extends ImmutablePureComponent {
           </a>
         </div>
 
-        <StatusContent status={this.props.status} />
+        <StatusContentContainer status={this.props.status} />
       </div>
     );
 

diff --git a/app/javascript/flavours/glitch/features/ui/components/boost_modal.js b/app/javascript/flavours/glitch/features/ui/components/boost_modal.js
index 8d9496bb9871fe08bd7344095b80c766e52f298f..11947a15fc67858289aecbc0adeffadbf6937553 100644 (file)
--- a/app/javascript/flavours/glitch/features/ui/components/boost_modal.js
+++ b/app/javascript/flavours/glitch/features/ui/components/boost_modal.js
@@ -4,7 +4,7 @@ import ImmutablePropTypes from 'react-immutable-proptypes';
 import PropTypes from 'prop-types';
 import { defineMessages, injectIntl, FormattedMessage } from 'react-intl';
 import Button from 'flavours/glitch/components/button';
-import StatusContent from 'flavours/glitch/components/status_content';
+import StatusContentContainer from 'flavours/glitch/containers/status_content_container';
 import Avatar from 'flavours/glitch/components/avatar';
 import RelativeTimestamp from 'flavours/glitch/components/relative_timestamp';
 import DisplayName from 'flavours/glitch/components/display_name';
@@ -102,7 +102,7 @@ class BoostModal extends ImmutablePureComponent {
               </a>
             </div>
 
-            <StatusContent status={status} />
+            <StatusContentContainer status={status} />
 
             {status.get('media_attachments').size > 0 && (
               <AttachmentList

diff --git a/app/javascript/flavours/glitch/features/ui/components/favourite_modal.js b/app/javascript/flavours/glitch/features/ui/components/favourite_modal.js
index 4d02be29b5c1e451263b463714442db2f86f05c9..0a88db2c04a967841387ecbd4f4146b43c293261 100644 (file)
--- a/app/javascript/flavours/glitch/features/ui/components/favourite_modal.js
+++ b/app/javascript/flavours/glitch/features/ui/components/favourite_modal.js
@@ -3,7 +3,7 @@ import ImmutablePropTypes from 'react-immutable-proptypes';
 import PropTypes from 'prop-types';
 import { defineMessages, injectIntl, FormattedMessage } from 'react-intl';
 import Button from 'flavours/glitch/components/button';
-import StatusContent from 'flavours/glitch/components/status_content';
+import StatusContentContainer from 'flavours/glitch/containers/status_content_container';
 import Avatar from 'flavours/glitch/components/avatar';
 import RelativeTimestamp from 'flavours/glitch/components/relative_timestamp';
 import DisplayName from 'flavours/glitch/components/display_name';
@@ -79,7 +79,7 @@ class FavouriteModal extends ImmutablePureComponent {
               </a>
             </div>
 
-            <StatusContent status={status} />
+            <StatusContentContainer status={status} />
 
             {status.get('media_attachments').size > 0 && (
               <AttachmentList