From 0ea02e608cf0ce0747a50130b9629af48ef3f7f9 Mon Sep 17 00:00:00 2001 From: fef Date: Sat, 3 Dec 2022 11:57:00 +0000 Subject: [PATCH] display external custom emoji reactions properly Using an emoji map was completely unnecessary in the first place, because the reaction list from the API response includes URLs for every custom emoji anyway. The reaction list now also contains a boolean field indicating whether it is an external custom emoji, which is required because people should only be able to react with Unicode emojis and local custom ones, not with custom emojis from other servers. --- .../flavours/glitch/components/status.js | 2 -- .../glitch/components/status_action_bar.js | 11 ++----- .../glitch/components/status_reactions.js | 33 ++++++++++--------- .../glitch/containers/status_container.js | 2 -- .../features/status/components/action_bar.js | 2 +- .../status/components/detailed_status.js | 2 -- .../flavours/glitch/features/status/index.js | 4 +-- .../flavours/glitch/selectors/index.js | 10 +----- app/serializers/rest/reaction_serializer.rb | 10 +++++- 9 files changed, 32 insertions(+), 44 deletions(-) diff --git a/app/javascript/flavours/glitch/components/status.js b/app/javascript/flavours/glitch/components/status.js index 3ef5f453fd..08cfbdada0 100644 --- a/app/javascript/flavours/glitch/components/status.js +++ b/app/javascript/flavours/glitch/components/status.js @@ -105,7 +105,6 @@ class Status extends ImmutablePureComponent { scrollKey: PropTypes.string, deployPictureInPicture: PropTypes.func, settings: ImmutablePropTypes.map.isRequired, - emojiMap: ImmutablePropTypes.map.isRequired, pictureInPicture: PropTypes.shape({ inUse: PropTypes.bool, available: PropTypes.bool, @@ -811,7 +810,6 @@ class Status extends ImmutablePureComponent { numVisible={visibleReactions} addReaction={this.props.onReactionAdd} removeReaction={this.props.onReactionRemove} - emojiMap={this.props.emojiMap} /> {!isCollapsed || !(muted || !settings.getIn(['collapsed', 'show_action_bar'])) ? ( diff --git a/app/javascript/flavours/glitch/components/status_action_bar.js b/app/javascript/flavours/glitch/components/status_action_bar.js index 36a3f83075..5129d3a68a 100644 --- a/app/javascript/flavours/glitch/components/status_action_bar.js +++ b/app/javascript/flavours/glitch/components/status_action_bar.js @@ -118,13 +118,7 @@ class StatusActionBar extends ImmutablePureComponent { } handleEmojiPick = data => { - const { signedIn } = this.context.identity; - - if (signedIn) { - this.props.onReactionAdd(this.props.status.get('id'), data.native.replace(/:/g, '')); - } else { - this.props.onInteractionModal('favourite', this.props.status); - } + this.props.onReactionAdd(this.props.status.get('id'), data.native.replace(/:/g, '')); } handleReblogClick = e => { @@ -212,6 +206,7 @@ class StatusActionBar extends ImmutablePureComponent { render () { const { status, intl, withDismiss, withCounters, showReplyCount, scrollKey } = this.props; + const { signedIn } = this.context.identity; const anonymousAccess = !me; const mutingConversation = status.get('muted'); @@ -311,7 +306,7 @@ class StatusActionBar extends ImmutablePureComponent { ); - const canReact = status.get('reactions').filter(r => r.get('count') > 0 && r.get('me')).size < maxReactions; + const canReact = signedIn && status.get('reactions').filter(r => r.get('count') > 0 && r.get('me')).size < maxReactions; const reactButton = ( ))} @@ -75,7 +73,6 @@ class Reaction extends ImmutablePureComponent { reaction: ImmutablePropTypes.map.isRequired, addReaction: PropTypes.func.isRequired, removeReaction: PropTypes.func.isRequired, - emojiMap: ImmutablePropTypes.map.isRequired, style: PropTypes.object, }; @@ -86,10 +83,12 @@ class Reaction extends ImmutablePureComponent { handleClick = () => { const { reaction, statusId, addReaction, removeReaction } = this.props; - if (reaction.get('me')) { - removeReaction(statusId, reaction.get('name')); - } else { - addReaction(statusId, reaction.get('name')); + if (!reaction.get('extern')) { + if (reaction.get('me')) { + removeReaction(statusId, reaction.get('name')); + } else { + addReaction(statusId, reaction.get('name')); + } } } @@ -109,7 +108,12 @@ class Reaction extends ImmutablePureComponent { style={this.props.style} > - + @@ -124,12 +128,13 @@ class Emoji extends React.PureComponent { static propTypes = { emoji: PropTypes.string.isRequired, - emojiMap: ImmutablePropTypes.map.isRequired, hovered: PropTypes.bool.isRequired, + url: PropTypes.string, + staticUrl: PropTypes.string, }; render() { - const { emoji, emojiMap, hovered } = this.props; + const { emoji, hovered, url, staticUrl } = this.props; if (unicodeMapping[emoji]) { const { filename, shortCode } = unicodeMapping[this.props.emoji]; @@ -144,10 +149,8 @@ class Emoji extends React.PureComponent { src={`${assetHost}/emoji/${filename}.svg`} /> ); - } else if (emojiMap.get(emoji)) { - const filename = (autoPlayGif || hovered) - ? emojiMap.getIn([emoji, 'url']) - : emojiMap.getIn([emoji, 'static_url']); + } else { + const filename = (autoPlayGif || hovered) ? url : staticUrl; const shortCode = `:${emoji}:`; return ( @@ -159,8 +162,6 @@ class Emoji extends React.PureComponent { src={filename} /> ); - } else { - return null; } } diff --git a/app/javascript/flavours/glitch/containers/status_container.js b/app/javascript/flavours/glitch/containers/status_container.js index c2fd15b17c..d68e059af4 100644 --- a/app/javascript/flavours/glitch/containers/status_container.js +++ b/app/javascript/flavours/glitch/containers/status_container.js @@ -45,7 +45,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 { makeCustomEmojiMap } from '../selectors'; const messages = defineMessages({ deleteConfirm: { id: 'confirmations.delete.confirm', defaultMessage: 'Delete' }, @@ -85,7 +84,6 @@ const makeMapStateToProps = () => { account: account || props.account, settings: state.get('local_settings'), prepend: prepend || props.prepend, - emojiMap: makeCustomEmojiMap(state), pictureInPicture: { inUse: state.getIn(['meta', 'layout']) !== 'mobile' && state.get('picture_in_picture').statusId === props.id, diff --git a/app/javascript/flavours/glitch/features/status/components/action_bar.js b/app/javascript/flavours/glitch/features/status/components/action_bar.js index 619c57685a..1ed89ea059 100644 --- a/app/javascript/flavours/glitch/features/status/components/action_bar.js +++ b/app/javascript/flavours/glitch/features/status/components/action_bar.js @@ -203,7 +203,7 @@ class ActionBar extends React.PureComponent { } } - const canReact = status.get('reactions').filter(r => r.get('count') > 0 && r.get('me')).size < maxReactions; + const canReact = signedIn && status.get('reactions').filter(r => r.get('count') > 0 && r.get('me')).size < maxReactions; const reactButton = (
diff --git a/app/javascript/flavours/glitch/features/status/index.js b/app/javascript/flavours/glitch/features/status/index.js index 910be59982..fd27692479 100644 --- a/app/javascript/flavours/glitch/features/status/index.js +++ b/app/javascript/flavours/glitch/features/status/index.js @@ -43,7 +43,7 @@ import { initMuteModal } from 'flavours/glitch/actions/mutes'; import { initBlockModal } from 'flavours/glitch/actions/blocks'; import { initReport } from 'flavours/glitch/actions/reports'; import { initBoostModal } from 'flavours/glitch/actions/boosts'; -import { makeCustomEmojiMap, makeGetStatus } from 'flavours/glitch/selectors'; +import { makeGetStatus } from 'flavours/glitch/selectors'; import ScrollContainer from 'flavours/glitch/containers/scroll_container'; import ColumnBackButton from 'flavours/glitch/components/column_back_button'; import ColumnHeader from '../../components/column_header'; @@ -148,7 +148,6 @@ const makeMapStateToProps = () => { askReplyConfirmation: state.getIn(['local_settings', 'confirm_before_clearing_draft']) && state.getIn(['compose', 'text']).trim().length !== 0, domain: state.getIn(['meta', 'domain']), usingPiP: state.get('picture_in_picture').statusId === props.params.statusId, - emojiMap: makeCustomEmojiMap(state), }; }; @@ -707,7 +706,6 @@ class Status extends ImmutablePureComponent { showMedia={this.state.showMedia} onToggleMediaVisibility={this.handleToggleMediaVisibility} usingPiP={usingPiP} - emojiMap={this.props.emojiMap} /> { return hidden && !(isSelf || followingOrRequested); }); - -export const makeCustomEmojiMap = createSelector( - [state => state.get('custom_emojis')], - items => items.reduce( - (map, emoji) => map.set(emoji.get('shortcode'), emoji), - ImmutableMap(), - ), -); diff --git a/app/serializers/rest/reaction_serializer.rb b/app/serializers/rest/reaction_serializer.rb index 1a5dca0183..007d3b5015 100644 --- a/app/serializers/rest/reaction_serializer.rb +++ b/app/serializers/rest/reaction_serializer.rb @@ -3,7 +3,7 @@ class REST::ReactionSerializer < ActiveModel::Serializer include RoutingHelper - attributes :name, :count + attributes :name, :count, :extern attribute :me, if: :current_user? attribute :url, if: :custom_emoji? @@ -21,6 +21,14 @@ class REST::ReactionSerializer < ActiveModel::Serializer object.custom_emoji.present? end + def extern + if custom_emoji? + object.custom_emoji.domain.present? + else + false + end + end + def url full_asset_url(object.custom_emoji.image.url) end