From 18462ee4b6b7b17fbffdba9756139c844f20689a Mon Sep 17 00:00:00 2001 From: Renaud Chaput Date: Mon, 21 Aug 2023 19:39:01 +0200 Subject: [PATCH] [Glitch] Remove hashtags from the last line of a status if it only contains hashtags Port 061fd66ee66afc8a5c7923ff7648f51a6da3fe7c to glitch-soc Signed-off-by: Claire --- .../components/__tests__/hashtag_bar.tsx | 184 +++++++++++++++ .../glitch/components/hashtag_bar.jsx | 50 ---- .../glitch/components/hashtag_bar.tsx | 222 ++++++++++++++++++ .../flavours/glitch/components/status.jsx | 10 +- .../glitch/components/status_content.jsx | 13 +- .../status/components/detailed_status.jsx | 7 +- 6 files changed, 428 insertions(+), 58 deletions(-) create mode 100644 app/javascript/flavours/glitch/components/__tests__/hashtag_bar.tsx delete mode 100644 app/javascript/flavours/glitch/components/hashtag_bar.jsx create mode 100644 app/javascript/flavours/glitch/components/hashtag_bar.tsx diff --git a/app/javascript/flavours/glitch/components/__tests__/hashtag_bar.tsx b/app/javascript/flavours/glitch/components/__tests__/hashtag_bar.tsx new file mode 100644 index 0000000000..c7db485d08 --- /dev/null +++ b/app/javascript/flavours/glitch/components/__tests__/hashtag_bar.tsx @@ -0,0 +1,184 @@ +import { fromJS } from 'immutable'; + +import type { StatusLike } from '../hashtag_bar'; +import { computeHashtagBarForStatus } from '../hashtag_bar'; + +function createStatus( + content: string, + hashtags: string[], + hasMedia = false, + spoilerText?: string, +) { + return fromJS({ + tags: hashtags.map((name) => ({ name })), + contentHtml: content, + media_attachments: hasMedia ? ['fakeMedia'] : [], + spoiler_text: spoilerText, + }) as unknown as StatusLike; // need to force the type here, as it is not properly defined +} + +describe('computeHashtagBarForStatus', () => { + it('does nothing when there are no tags', () => { + const status = createStatus('

Simple text

', []); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual([]); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

Simple text

"`, + ); + }); + + it('displays out of band hashtags in the bar', () => { + const status = createStatus( + '

Simple text #hashtag

', + ['hashtag', 'test'], + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual(['test']); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

Simple text #hashtag

"`, + ); + }); + + it('extract tags from the last line', () => { + const status = createStatus( + '

Simple text

#hashtag

', + ['hashtag'], + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual(['hashtag']); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

Simple text

"`, + ); + }); + + it('does not include tags from content', () => { + const status = createStatus( + '

Simple text with a #hashtag

#hashtag

', + ['hashtag'], + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual([]); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

Simple text with a #hashtag

"`, + ); + }); + + it('works with one line status and hashtags', () => { + const status = createStatus( + '

#test. And another #hashtag

', + ['hashtag', 'test'], + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual([]); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

#test. And another #hashtag

"`, + ); + }); + + it('de-duplicate accentuated characters with case differences', () => { + const status = createStatus( + '

Text

#éaa #Éaa

', + ['éaa'], + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual(['Éaa']); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

Text

"`, + ); + }); + + it('does not display in bar a hashtag in content with a case difference', () => { + const status = createStatus( + '

Text #Éaa

#éaa

', + ['éaa'], + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual([]); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

Text #Éaa

"`, + ); + }); + + it('does not modify a status with a line of hashtags only', () => { + const status = createStatus( + '

#test #hashtag

', + ['test', 'hashtag'], + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual([]); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

#test #hashtag

"`, + ); + }); + + it('puts the hashtags in the bar if a status content has hashtags in the only line and has a media', () => { + const status = createStatus( + '

This is my content! #hashtag

', + ['hashtag'], + true, + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual([]); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

This is my content! #hashtag

"`, + ); + }); + + it('puts the hashtags in the bar if a status content is only hashtags and has a media', () => { + const status = createStatus( + '

#test #hashtag

', + ['test', 'hashtag'], + true, + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual(['test', 'hashtag']); + expect(statusContentProps.statusContent).toMatchInlineSnapshot(`""`); + }); + + it('does not use the hashtag bar if the status content is only hashtags, has a CW and a media', () => { + const status = createStatus( + '

#test #hashtag

', + ['test', 'hashtag'], + true, + 'My CW text', + ); + + const { hashtagsInBar, statusContentProps } = + computeHashtagBarForStatus(status); + + expect(hashtagsInBar).toEqual([]); + expect(statusContentProps.statusContent).toMatchInlineSnapshot( + `"

#test #hashtag

"`, + ); + }); +}); diff --git a/app/javascript/flavours/glitch/components/hashtag_bar.jsx b/app/javascript/flavours/glitch/components/hashtag_bar.jsx deleted file mode 100644 index 3c7e24228d..0000000000 --- a/app/javascript/flavours/glitch/components/hashtag_bar.jsx +++ /dev/null @@ -1,50 +0,0 @@ -import PropTypes from 'prop-types'; -import { useMemo, useState, useCallback } from 'react'; - -import { FormattedMessage } from 'react-intl'; - -import { Link } from 'react-router-dom'; - -import ImmutablePropTypes from 'react-immutable-proptypes'; - -const domParser = new DOMParser(); - -// About two lines on desktop -const VISIBLE_HASHTAGS = 7; - -export const HashtagBar = ({ hashtags, text }) => { - const renderedHashtags = useMemo(() => { - const body = domParser.parseFromString(text, 'text/html').documentElement; - return [].filter.call(body.querySelectorAll('a[href]'), link => link.textContent[0] === '#' || (link.previousSibling?.textContent?.[link.previousSibling.textContent.length - 1] === '#')).map(node => node.textContent); - }, [text]); - - const invisibleHashtags = useMemo(() => ( - hashtags.filter(hashtag => !renderedHashtags.some(textContent => textContent.localeCompare(`#${hashtag.get('name')}`, undefined, { sensitivity: 'accent' }) === 0 || textContent.localeCompare(hashtag.get('name'), undefined, { sensitivity: 'accent' }) === 0)) - ), [hashtags, renderedHashtags]); - - const [expanded, setExpanded] = useState(false); - const handleClick = useCallback(() => setExpanded(true), []); - - if (invisibleHashtags.isEmpty()) { - return null; - } - - const revealedHashtags = expanded ? invisibleHashtags : invisibleHashtags.take(VISIBLE_HASHTAGS); - - return ( -
- {revealedHashtags.map(hashtag => ( - - #{hashtag.get('name')} - - ))} - - {!expanded && invisibleHashtags.size > VISIBLE_HASHTAGS && } -
- ); -}; - -HashtagBar.propTypes = { - hashtags: ImmutablePropTypes.list, - text: PropTypes.string, -}; diff --git a/app/javascript/flavours/glitch/components/hashtag_bar.tsx b/app/javascript/flavours/glitch/components/hashtag_bar.tsx new file mode 100644 index 0000000000..8781c26630 --- /dev/null +++ b/app/javascript/flavours/glitch/components/hashtag_bar.tsx @@ -0,0 +1,222 @@ +import { useState, useCallback } from 'react'; + +import { FormattedMessage } from 'react-intl'; + +import { Link } from 'react-router-dom'; + +import type { List, Record } from 'immutable'; + +import { groupBy, minBy } from 'lodash'; + +import { getStatusContent } from './status_content'; + +// About two lines on desktop +const VISIBLE_HASHTAGS = 7; + +// Those types are not correct, they need to be replaced once this part of the state is typed +export type TagLike = Record<{ name: string }>; +export type StatusLike = Record<{ + tags: List; + contentHTML: string; + media_attachments: List; + spoiler_text?: string; +}>; + +function normalizeHashtag(hashtag: string) { + if (hashtag && hashtag.startsWith('#')) return hashtag.slice(1); + else return hashtag; +} + +function isNodeLinkHashtag(element: Node): element is HTMLLinkElement { + return ( + element instanceof HTMLAnchorElement && + // it may be a starting with a hashtag + (element.textContent?.[0] === '#' || + // or a # + element.previousSibling?.textContent?.[ + element.previousSibling.textContent.length - 1 + ] === '#') + ); +} + +/** + * Removes duplicates from an hashtag list, case-insensitive, keeping only the best one + * "Best" here is defined by the one with the more casing difference (ie, the most camel-cased one) + * @param hashtags The list of hashtags + * @returns The input hashtags, but with only 1 occurence of each (case-insensitive) + */ +function uniqueHashtagsWithCaseHandling(hashtags: string[]) { + const groups = groupBy(hashtags, (tag) => + tag.normalize('NFKD').toLowerCase(), + ); + + return Object.values(groups).map((tags) => { + if (tags.length === 1) return tags[0]; + + // The best match is the one where we have the less difference between upper and lower case letter count + const best = minBy(tags, (tag) => { + const upperCase = Array.from(tag).reduce( + (acc, char) => (acc += char.toUpperCase() === char ? 1 : 0), + 0, + ); + + const lowerCase = tag.length - upperCase; + + return Math.abs(lowerCase - upperCase); + }); + + return best ?? tags[0]; + }); +} + +// Create the collator once, this is much more efficient +const collator = new Intl.Collator(undefined, { sensitivity: 'accent' }); +function localeAwareInclude(collection: string[], value: string) { + return collection.find((item) => collator.compare(item, value) === 0); +} + +// We use an intermediate function here to make it easier to test +export function computeHashtagBarForStatus(status: StatusLike): { + statusContentProps: { statusContent: string }; + hashtagsInBar: string[]; +} { + let statusContent = getStatusContent(status); + + const tagNames = status + .get('tags') + .map((tag) => tag.get('name')) + .toJS(); + + // this is returned if we stop the processing early, it does not change what is displayed + const defaultResult = { + statusContentProps: { statusContent }, + hashtagsInBar: [], + }; + + // return early if this status does not have any tags + if (tagNames.length === 0) return defaultResult; + + const template = document.createElement('template'); + template.innerHTML = statusContent.trim(); + + const lastChild = template.content.lastChild; + + if (!lastChild) return defaultResult; + + template.content.removeChild(lastChild); + const contentWithoutLastLine = template; + + // First, try to parse + const contentHashtags = Array.from( + contentWithoutLastLine.content.querySelectorAll('a[href]'), + ).reduce((result, link) => { + if (isNodeLinkHashtag(link)) { + if (link.textContent) result.push(normalizeHashtag(link.textContent)); + } + return result; + }, []); + + // Now we parse the last line, and try to see if it only contains hashtags + const lastLineHashtags: string[] = []; + // try to see if the last line is only hashtags + let onlyHashtags = true; + + Array.from(lastChild.childNodes).forEach((node) => { + if (isNodeLinkHashtag(node) && node.textContent) { + const normalized = normalizeHashtag(node.textContent); + + if (!localeAwareInclude(tagNames, normalized)) { + // stop here, this is not a real hashtag, so consider it as text + onlyHashtags = false; + return; + } + + if (!localeAwareInclude(contentHashtags, normalized)) + // only add it if it does not appear in the rest of the content + lastLineHashtags.push(normalized); + } else if (node.nodeType !== Node.TEXT_NODE || node.nodeValue?.trim()) { + // not a space + onlyHashtags = false; + } + }); + + const hashtagsInBar = tagNames.filter( + (tag) => + // the tag does not appear at all in the status content, it is an out-of-band tag + !localeAwareInclude(contentHashtags, tag) && + !localeAwareInclude(lastLineHashtags, tag), + ); + + const isOnlyOneLine = contentWithoutLastLine.content.childElementCount === 0; + const hasMedia = status.get('media_attachments').size > 0; + const hasSpoiler = !!status.get('spoiler_text'); + + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- due to https://github.com/microsoft/TypeScript/issues/9998 + if (onlyHashtags && ((hasMedia && !hasSpoiler) || !isOnlyOneLine)) { + // if the last line only contains hashtags, and we either: + // - have other content in the status + // - dont have other content, but a media and no CW. If it has a CW, then we do not remove the content to avoid having an empty content behind the CW button + statusContent = contentWithoutLastLine.innerHTML; + // and add the tags to the bar + hashtagsInBar.push(...lastLineHashtags); + } + + return { + statusContentProps: { statusContent }, + hashtagsInBar: uniqueHashtagsWithCaseHandling(hashtagsInBar), + }; +} + +/** + * This function will process a status to, at the same time (avoiding parsing it twice): + * - build the HashtagBar for this status + * - remove the last-line hashtags from the status content + * @param status The status to process + * @returns Props to be passed to the component, and the hashtagBar to render + */ +export function getHashtagBarForStatus(status: StatusLike) { + const { statusContentProps, hashtagsInBar } = + computeHashtagBarForStatus(status); + + return { + statusContentProps, + hashtagBar: , + }; +} + +const HashtagBar: React.FC<{ + hashtags: string[]; +}> = ({ hashtags }) => { + const [expanded, setExpanded] = useState(false); + const handleClick = useCallback(() => { + setExpanded(true); + }, []); + + if (hashtags.length === 0) { + return null; + } + + const revealedHashtags = expanded + ? hashtags + : hashtags.slice(0, VISIBLE_HASHTAGS - 1); + + return ( +
+ {revealedHashtags.map((hashtag) => ( + + #{hashtag} + + ))} + + {!expanded && hashtags.length > VISIBLE_HASHTAGS && ( + + )} +
+ ); +}; diff --git a/app/javascript/flavours/glitch/components/status.jsx b/app/javascript/flavours/glitch/components/status.jsx index c43e913c66..e5693a94df 100644 --- a/app/javascript/flavours/glitch/components/status.jsx +++ b/app/javascript/flavours/glitch/components/status.jsx @@ -20,8 +20,8 @@ import Bundle from '../features/ui/components/bundle'; import { MediaGallery, Video, Audio } from '../features/ui/util/async-components'; import { displayMedia } from '../initial_state'; -import { HashtagBar } from './hashtag_bar'; import AttachmentList from './attachment_list'; +import { getHashtagBarForStatus } from './hashtag_bar'; import StatusActionBar from './status_action_bar'; import StatusContent from './status_content'; import StatusHeader from './status_header'; @@ -740,10 +740,6 @@ class Status extends ImmutablePureComponent { contentMediaIcons.push('tasks'); } - media.push( - - ); - // Here we prepare extra data-* attributes for CSS selectors. // Users can use those for theming, hiding avatars etc via UserStyle const selectorAttribs = { @@ -784,6 +780,9 @@ class Status extends ImmutablePureComponent { muted, }, 'focusable'); + const {statusContentProps, hashtagBar} = getHashtagBarForStatus(status); + media.push(hashtagBar); + return (
{!isCollapsed || !(muted || !settings.getIn(['collapsed', 'show_action_bar'])) ? ( diff --git a/app/javascript/flavours/glitch/components/status_content.jsx b/app/javascript/flavours/glitch/components/status_content.jsx index 03b2799168..67e5d53d83 100644 --- a/app/javascript/flavours/glitch/components/status_content.jsx +++ b/app/javascript/flavours/glitch/components/status_content.jsx @@ -69,6 +69,15 @@ const isLinkMisleading = (link) => { return !(textMatchesTarget(text, origin, host) || textMatchesTarget(text.toLowerCase(), origin, host)); }; +/** + * + * @param {any} status + * @returns {string} + */ +export function getStatusContent(status) { + return status.getIn(['translation', 'contentHtml']) || status.get('contentHtml'); +} + class TranslateButton extends PureComponent { static propTypes = { @@ -118,6 +127,7 @@ class StatusContent extends PureComponent { static propTypes = { status: ImmutablePropTypes.map.isRequired, + statusContent: PropTypes.string, expanded: PropTypes.bool, collapsed: PropTypes.bool, onExpandedToggle: PropTypes.func, @@ -327,6 +337,7 @@ class StatusContent extends PureComponent { tagLinks, rewriteMentions, intl, + statusContent, } = this.props; const hidden = this.props.onExpandedToggle ? !this.props.expanded : this.state.hidden; @@ -334,7 +345,7 @@ class StatusContent extends PureComponent { const targetLanguages = this.props.languages?.get(status.get('language') || 'und'); const renderTranslate = this.props.onTranslate && this.context.identity.signedIn && ['public', 'unlisted'].includes(status.get('visibility')) && status.get('search_index').trim().length > 0 && targetLanguages?.includes(contentLocale); - const content = { __html: status.getIn(['translation', 'contentHtml']) || status.get('contentHtml') }; + const content = { __html: statusContent ?? getStatusContent(status) }; const spoilerContent = { __html: status.getIn(['translation', 'spoilerHtml']) || status.get('spoilerHtml') }; const language = status.getIn(['translation', 'language']) || status.get('language'); const classNames = classnames('status__content', { 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 f8134cf5ee..99e6e63e04 100644 --- a/app/javascript/flavours/glitch/features/status/components/detailed_status.jsx +++ b/app/javascript/flavours/glitch/features/status/components/detailed_status.jsx @@ -11,7 +11,7 @@ import ImmutablePureComponent from 'react-immutable-pure-component'; import { AnimatedNumber } from 'flavours/glitch/components/animated_number'; import AttachmentList from 'flavours/glitch/components/attachment_list'; import EditedTimestamp from 'flavours/glitch/components/edited_timestamp'; -import { HashtagBar } from 'flavours/glitch/components/hashtag_bar'; +import { getHashtagBarForStatus } from 'flavours/glitch/components/hashtag_bar'; import { Icon } from 'flavours/glitch/components/icon'; import PictureInPicturePlaceholder from 'flavours/glitch/components/picture_in_picture_placeholder'; import VisibilityIcon from 'flavours/glitch/components/status_visibility_icon'; @@ -304,6 +304,8 @@ class DetailedStatus extends ImmutablePureComponent { ); } + const {statusContentProps, hashtagBar} = getHashtagBarForStatus(status); + return (