From a5c96afeac2c4410ebdc4b769dd9e1b6008648d5 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.jsx | 2 -- .../glitch/components/status_action_bar.jsx | 11 ++----- .../glitch/components/status_reactions.js | 33 ++++++++++--------- .../glitch/containers/status_container.js | 2 -- .../features/status/components/action_bar.jsx | 2 +- .../status/components/detailed_status.jsx | 2 -- .../flavours/glitch/features/status/index.jsx | 5 ++- .../flavours/glitch/selectors/index.js | 8 ----- app/serializers/rest/reaction_serializer.rb | 10 +++++- 9 files changed, 33 insertions(+), 42 deletions(-) diff --git a/app/javascript/flavours/glitch/components/status.jsx b/app/javascript/flavours/glitch/components/status.jsx index 575aad991e..3162929368 100644 --- a/app/javascript/flavours/glitch/components/status.jsx +++ b/app/javascript/flavours/glitch/components/status.jsx @@ -116,7 +116,6 @@ class Status extends ImmutablePureComponent { scrollKey: PropTypes.string, deployPictureInPicture: PropTypes.func, settings: ImmutablePropTypes.map.isRequired, - emojiMap: ImmutablePropTypes.map.isRequired, pictureInPicture: ImmutablePropTypes.contains({ inUse: PropTypes.bool, available: PropTypes.bool, @@ -843,7 +842,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.jsx b/app/javascript/flavours/glitch/components/status_action_bar.jsx index cfc3e17bd1..6a72f54b7c 100644 --- a/app/javascript/flavours/glitch/components/status_action_bar.jsx +++ b/app/javascript/flavours/glitch/components/status_action_bar.jsx @@ -122,13 +122,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 => { @@ -216,7 +210,6 @@ class StatusActionBar extends ImmutablePureComponent { render () { const { status, intl, withDismiss, withCounters, showReplyCount, scrollKey } = this.props; const { permissions } = this.context.identity; - const anonymousAccess = !me; const mutingConversation = status.get('muted'); const publicStatus = ['public', 'unlisted'].includes(status.get('visibility')); @@ -317,7 +310,7 @@ class StatusActionBar extends ImmutablePureComponent { ); - const canReact = status.get('reactions').filter(r => r.get('count') > 0 && r.get('me')).size < maxReactions; + const canReact = permissions && 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 8b3878f033..07d19ecfff 100644 --- a/app/javascript/flavours/glitch/containers/status_container.js +++ b/app/javascript/flavours/glitch/containers/status_container.js @@ -47,7 +47,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' }, @@ -91,7 +90,6 @@ const makeMapStateToProps = () => { account: account || props.account, settings: state.get('local_settings'), prepend: prepend || props.prepend, - emojiMap: makeCustomEmojiMap(state), pictureInPicture: getPictureInPicture(state, props), }; }; diff --git a/app/javascript/flavours/glitch/features/status/components/action_bar.jsx b/app/javascript/flavours/glitch/features/status/components/action_bar.jsx index 69a674f623..588d59b1f5 100644 --- a/app/javascript/flavours/glitch/features/status/components/action_bar.jsx +++ b/app/javascript/flavours/glitch/features/status/components/action_bar.jsx @@ -211,7 +211,7 @@ class ActionBar extends 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.jsx b/app/javascript/flavours/glitch/features/status/index.jsx index 505d5c9b32..c2fe260211 100644 --- a/app/javascript/flavours/glitch/features/status/index.jsx +++ b/app/javascript/flavours/glitch/features/status/index.jsx @@ -156,7 +156,6 @@ const makeMapStateToProps = () => { askReplyConfirmation: state.getIn(['local_settings', 'confirm_before_clearing_draft']) && state.getIn(['compose', 'text']).trim().length !== 0, domain: state.getIn(['meta', 'domain']), pictureInPicture: getPictureInPicture(state, { id: props.params.statusId }), - emojiMap: makeCustomEmojiMap(state), }; }; @@ -747,8 +746,12 @@ class Status extends ImmutablePureComponent { domain={domain} showMedia={this.state.showMedia} onToggleMediaVisibility={this.handleToggleMediaVisibility} +<<<<<<< HEAD pictureInPicture={pictureInPicture} emojiMap={this.props.emojiMap} +======= + usingPiP={usingPiP} +>>>>>>> f0197c80d (display external custom emoji reactions properly) /> state.getIn(['status_lists', type, 'items']), ], (items) => items.toList()); - -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