From abf7c5b85016ec1c660ef8c6598f4b8acbb8c41c Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Fri, 16 Mar 2018 17:19:38 +0100 Subject: [PATCH 1/6] [Glitch] Fix column headers accessibility Port 08e4c78e78358c2847967e9cc34b4a6497be97e2 to glitch-soc. SCSS fix inspired by 69f13e7bca90f4855e05a49dd69bba66f843134a. --- .../glitch/components/column_header.js | 19 ++++++++----------- .../glitch/styles/components/columns.scss | 4 +--- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/app/javascript/flavours/glitch/components/column_header.js b/app/javascript/flavours/glitch/components/column_header.js index ae90b6f81ff..e4296ecefe1 100644 --- a/app/javascript/flavours/glitch/components/column_header.js +++ b/app/javascript/flavours/glitch/components/column_header.js @@ -28,7 +28,6 @@ export default class ColumnHeader extends React.PureComponent { active: PropTypes.bool, localSettings : ImmutablePropTypes.map, multiColumn: PropTypes.bool, - focusable: PropTypes.bool, showBackButton: PropTypes.bool, notifCleaning: PropTypes.bool, // true only for the notification column notifCleaningActive: PropTypes.bool, @@ -41,10 +40,6 @@ export default class ColumnHeader extends React.PureComponent { intl: PropTypes.object.isRequired, }; - static defaultProps = { - focusable: true, - } - state = { collapsed: true, animating: false, @@ -91,7 +86,7 @@ export default class ColumnHeader extends React.PureComponent { } render () { - const { intl, icon, active, children, pinned, onPin, multiColumn, focusable, showBackButton, intl: { formatMessage }, notifCleaning, notifCleaningActive } = this.props; + const { intl, icon, active, children, pinned, onPin, multiColumn, showBackButton, intl: { formatMessage }, notifCleaning, notifCleaningActive } = this.props; const { collapsed, animating, animatingNCD } = this.state; let title = this.props.title; @@ -172,11 +167,13 @@ export default class ColumnHeader extends React.PureComponent { return (
-

- - - {title} - +

+
{backButton} { notifCleaning ? ( diff --git a/app/javascript/flavours/glitch/styles/components/columns.scss b/app/javascript/flavours/glitch/styles/components/columns.scss index 5dcfab4d1a9..933aea9199b 100644 --- a/app/javascript/flavours/glitch/styles/components/columns.scss +++ b/app/javascript/flavours/glitch/styles/components/columns.scss @@ -243,7 +243,6 @@ .column-header { display: flex; font-size: 16px; - padding: 15px; background: lighten($ui-base-color, 4%); flex: 0 0 auto; cursor: pointer; @@ -255,7 +254,7 @@ & > button { margin: 0; border: none; - padding: 15px 0 15px 15px; + padding: 15px; color: inherit; background: transparent; font: inherit; @@ -303,7 +302,6 @@ .column-header__buttons { height: 48px; display: flex; - margin: -15px; margin-left: 0; } From 63d671b18f010d5f60421b0bd2acd8eeff45169d Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Fri, 16 Mar 2018 17:21:28 +0100 Subject: [PATCH 2/6] [Glitch] Fix button hiding when header title is too long Port ffb2b8ef8c3c7cd6f57860240378fac8d5964105 to glitch-soc --- app/javascript/flavours/glitch/components/column_header.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/javascript/flavours/glitch/components/column_header.js b/app/javascript/flavours/glitch/components/column_header.js index e4296ecefe1..cd977c4cae1 100644 --- a/app/javascript/flavours/glitch/components/column_header.js +++ b/app/javascript/flavours/glitch/components/column_header.js @@ -170,9 +170,7 @@ export default class ColumnHeader extends React.PureComponent {

{backButton} From 259bc9840b061fa466b203035b2941efd5a8907f Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Wed, 28 Mar 2018 17:13:13 +0200 Subject: [PATCH 3/6] [Glitch] Fix style of legacy column headers Backports daefbd66a653b11a17be73b7b0a4ca200b466cec --- .../features/ui/components/column_header.js | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/app/javascript/flavours/glitch/features/ui/components/column_header.js b/app/javascript/flavours/glitch/features/ui/components/column_header.js index af195ea9c07..e8bdd8054f4 100644 --- a/app/javascript/flavours/glitch/features/ui/components/column_header.js +++ b/app/javascript/flavours/glitch/features/ui/components/column_header.js @@ -1,5 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; +import classNames from 'classnames'; export default class ColumnHeader extends React.PureComponent { @@ -16,19 +17,20 @@ export default class ColumnHeader extends React.PureComponent { } render () { - const { type, active, columnHeaderId } = this.props; + const { icon, type, active, columnHeaderId } = this.props; + let iconElement = ''; - let icon = ''; - - if (this.props.icon) { - icon = ; + if (icon) { + iconElement = ; } return ( -
- {icon} - {type} -
+

+ +

); } From 2888f74c1219e53e0aca28263ea8289d17264c21 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Wed, 28 Mar 2018 15:40:34 +0200 Subject: [PATCH 4/6] Refactor the infamous three-valued boolean into two booleans, trying to simplify the logic --- .../flavours/glitch/components/status.js | 72 ++++++++++--------- .../glitch/components/status_content.js | 5 +- .../glitch/components/status_icons.js | 6 +- .../status/components/detailed_status.js | 1 + .../flavours/glitch/features/status/index.js | 6 +- 5 files changed, 49 insertions(+), 41 deletions(-) diff --git a/app/javascript/flavours/glitch/components/status.js b/app/javascript/flavours/glitch/components/status.js index cf82c9ac6bb..2c011bb2a89 100644 --- a/app/javascript/flavours/glitch/components/status.js +++ b/app/javascript/flavours/glitch/components/status.js @@ -50,7 +50,8 @@ export default class Status extends ImmutablePureComponent { }; state = { - isExpanded: null, + isExpanded: false, + isCollapsed: false, autoCollapsed: false, } @@ -71,6 +72,7 @@ export default class Status extends ImmutablePureComponent { updateOnStates = [ 'isExpanded', + 'isCollapsed', ] // If our settings have changed to disable collapsed statuses, then we @@ -83,18 +85,18 @@ export default class Status extends ImmutablePureComponent { // uncollapse our status accordingly. componentWillReceiveProps (nextProps) { if (!nextProps.settings.getIn(['collapsed', 'enabled'])) { - if (this.state.isExpanded === false) { - this.setExpansion(null); + if (this.state.isCollapsed) { + this.setCollapsed(false); } } else if ( nextProps.collapse !== this.props.collapse && nextProps.collapse !== undefined - ) this.setExpansion(nextProps.collapse ? false : null); + ) this.setCollapsed(nextProps.collapse); } // When mounting, we just check to see if our status should be collapsed, // and collapse it if so. We don't need to worry about whether collapsing - // is enabled here, because `setExpansion()` already takes that into + // is enabled here, because `setCollapsed()` already takes that into // account. // The cases where a status should be collapsed are: @@ -138,7 +140,7 @@ export default class Status extends ImmutablePureComponent { return false; } }()) { - this.setExpansion(false); + this.setCollapsed(true); // Hack to fix timeline jumps on second rendering when auto-collapsing this.setState({ autoCollapsed: true }); } @@ -164,23 +166,26 @@ export default class Status extends ImmutablePureComponent { } } - // `setExpansion()` sets the value of `isExpanded` in our state. It takes - // one argument, `value`, which gives the desired value for `isExpanded`. - // The default for this argument is `null`. + // `setCollapsed()` sets the value of `isCollapsed` in our state, that is, + // whether the toot is collapsed or not. - // `setExpansion()` automatically checks for us whether toot collapsing + // `setCollapsed()` automatically checks for us whether toot collapsing // is enabled, so we don't have to. + setCollapsed = (value) => { + if (this.props.settings.getIn(['collapsed', 'enabled'])) { + this.setState({ isCollapsed: value }); + if (value) { + this.setExpansion(false); + } + } else { + this.setState({ isCollapsed: false }); + } + } + setExpansion = (value) => { - switch (true) { - case value === undefined || value === null: - this.setState({ isExpanded: null }); - break; - case !value && this.props.settings.getIn(['collapsed', 'enabled']): - this.setState({ isExpanded: false }); - break; - case !!value: - this.setState({ isExpanded: true }); - break; + this.setState({ isExpanded: value }); + if (value) { + this.setCollapsed(false); } } @@ -192,7 +197,7 @@ export default class Status extends ImmutablePureComponent { parseClick = (e, destination) => { const { router } = this.context; const { status } = this.props; - const { isExpanded } = this.state; + const { isCollapsed } = this.state; if (!router) return; if (destination === undefined) { destination = `/statuses/${ @@ -200,9 +205,9 @@ export default class Status extends ImmutablePureComponent { }`; } if (e.button === 0) { - if (isExpanded === false) this.setExpansion(null); + if (isCollapsed) this.setCollapsed(false); else if (e.shiftKey) { - this.setExpansion(false); + this.setCollapsed(true); document.getSelection().removeAllRanges(); } else router.history.push(destination); e.preventDefault(); @@ -219,7 +224,7 @@ export default class Status extends ImmutablePureComponent { handleExpandedToggle = () => { if (this.props.status.get('spoiler_text')) { - this.setExpansion(this.state.isExpanded ? null : true); + this.setExpansion(!this.state.isExpanded); } }; @@ -278,6 +283,7 @@ export default class Status extends ImmutablePureComponent { handleRef, parseClick, setExpansion, + setCollapsed, } = this; const { router } = this.context; const { @@ -294,7 +300,7 @@ export default class Status extends ImmutablePureComponent { hidden, ...other } = this.props; - const { isExpanded } = this.state; + const { isExpanded, isCollapsed } = this.state; let background = null; let attachments = null; let media = null; @@ -413,8 +419,8 @@ export default class Status extends ImmutablePureComponent { }; const computedClass = classNames('status', `status-${status.get('visibility')}`, { - collapsed: isExpanded === false, - 'has-background': isExpanded === false && background, + collapsed: isCollapsed, + 'has-background': isCollapsed && background, muted, }, 'focusable'); @@ -422,7 +428,7 @@ export default class Status extends ImmutablePureComponent {
) : null} - {!muted || isExpanded !== false ? ( + {!muted || !isCollapsed ? ( ) : null} @@ -450,8 +456,8 @@ export default class Status extends ImmutablePureComponent { status={status} mediaIcon={mediaIcon} collapsible={settings.getIn(['collapsed', 'enabled'])} - collapsed={isExpanded === false} - setExpansion={setExpansion} + collapsed={isCollapsed} + setCollapsed={setCollapsed} /> - {isExpanded !== false || !muted ? ( + {!isCollapsed || !muted ? ( { - if (this.props.expanded === false) { + if (this.props.collapsed) { if (this.props.parseClick) this.props.parseClick(e); } } @@ -112,7 +113,7 @@ export default class StatusContent extends React.PureComponent { e.preventDefault(); if (this.props.setExpansion) { - this.props.setExpansion(this.props.expanded ? null : true); + this.props.setExpansion(!this.props.expanded); } else { this.setState({ hidden: !this.state.hidden }); } diff --git a/app/javascript/flavours/glitch/components/status_icons.js b/app/javascript/flavours/glitch/components/status_icons.js index 4f585175c81..e354c45e417 100644 --- a/app/javascript/flavours/glitch/components/status_icons.js +++ b/app/javascript/flavours/glitch/components/status_icons.js @@ -22,15 +22,15 @@ export default class StatusIcons extends React.PureComponent { mediaIcon: PropTypes.string, collapsible: PropTypes.bool, collapsed: PropTypes.bool, - setExpansion: PropTypes.func.isRequired, + setCollapsed: PropTypes.func.isRequired, intl: PropTypes.object.isRequired, }; // Handles clicks on collapsed button handleCollapsedClick = (e) => { - const { collapsed, setExpansion } = this.props; + const { collapsed, setCollapsed } = this.props; if (e.button === 0) { - setExpansion(collapsed ? null : false); + setCollapsed(!collapsed); e.preventDefault(); } } 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 066499da8c7..dddfd36b07f 100644 --- a/app/javascript/flavours/glitch/features/status/components/detailed_status.js +++ b/app/javascript/flavours/glitch/features/status/components/detailed_status.js @@ -114,6 +114,7 @@ export default class DetailedStatus extends ImmutablePureComponent { media={media} mediaIcon={mediaIcon} expanded={expanded} + collapsed={false} setExpansion={setExpansion} /> diff --git a/app/javascript/flavours/glitch/features/status/index.js b/app/javascript/flavours/glitch/features/status/index.js index 717f99d4af4..a1783d94ac1 100644 --- a/app/javascript/flavours/glitch/features/status/index.js +++ b/app/javascript/flavours/glitch/features/status/index.js @@ -76,7 +76,7 @@ export default class Status extends ImmutablePureComponent { state = { fullscreen: false, - isExpanded: null, + isExpanded: false, }; componentWillMount () { @@ -96,7 +96,7 @@ export default class Status extends ImmutablePureComponent { handleExpandedToggle = () => { if (this.props.status.get('spoiler_text')) { - this.setExpansion(this.state.isExpanded ? null : true); + this.setExpansion(!this.state.isExpanded); } }; @@ -292,7 +292,7 @@ export default class Status extends ImmutablePureComponent { } setExpansion = value => { - this.setState({ isExpanded: value ? true : null }); + this.setState({ isExpanded: value }); } setRef = c => { From 64d8d0464dedeabf09b5a651151f177dba2053df Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Wed, 28 Mar 2018 19:56:46 +0200 Subject: [PATCH 5/6] Add show more/less toggle for entire threads in web UI Inspired from b6003afcdb1b89eb967a2b211e3b4e26aed9ac9d but using component properties instead of redux store for hidden/revealed state. --- .../glitch/components/column_header.js | 27 ++++++++++++------- .../flavours/glitch/components/status.js | 8 ++++-- .../glitch/components/status_content.js | 8 +++--- .../status/components/detailed_status.js | 6 +++-- .../flavours/glitch/features/status/index.js | 21 ++++++++++++--- .../glitch/styles/components/columns.scss | 4 +++ 6 files changed, 54 insertions(+), 20 deletions(-) diff --git a/app/javascript/flavours/glitch/components/column_header.js b/app/javascript/flavours/glitch/components/column_header.js index cd977c4cae1..bfad6467d54 100644 --- a/app/javascript/flavours/glitch/components/column_header.js +++ b/app/javascript/flavours/glitch/components/column_header.js @@ -23,11 +23,12 @@ export default class ColumnHeader extends React.PureComponent { static propTypes = { intl: PropTypes.object.isRequired, - title: PropTypes.node.isRequired, - icon: PropTypes.string.isRequired, + title: PropTypes.node, + icon: PropTypes.string, active: PropTypes.bool, localSettings : ImmutablePropTypes.map, multiColumn: PropTypes.bool, + extraButton: PropTypes.node, showBackButton: PropTypes.bool, notifCleaning: PropTypes.bool, // true only for the notification column notifCleaningActive: PropTypes.bool, @@ -86,7 +87,7 @@ export default class ColumnHeader extends React.PureComponent { } render () { - const { intl, icon, active, children, pinned, onPin, multiColumn, showBackButton, intl: { formatMessage }, notifCleaning, notifCleaningActive } = this.props; + const { intl, icon, active, children, pinned, onPin, multiColumn, extraButton, showBackButton, intl: { formatMessage }, notifCleaning, notifCleaningActive } = this.props; const { collapsed, animating, animatingNCD } = this.state; let title = this.props.title; @@ -162,18 +163,26 @@ export default class ColumnHeader extends React.PureComponent { } if (children || multiColumn) { - collapseButton = ; + collapseButton = ; } + const hasTitle = icon && title; + return (

- + {hasTitle && ( + + )} + + {!hasTitle && backButton} +
- {backButton} + {hasTitle && backButton} + {extraButton} { notifCleaning ? ( + )} + />
@@ -374,7 +389,7 @@ export default class Status extends ImmutablePureComponent { onOpenVideo={this.handleOpenVideo} onOpenMedia={this.handleOpenMedia} expanded={isExpanded} - setExpansion={setExpansion} + onToggleHidden={this.handleExpandedToggle} /> .column-header__back-button { + color: $ui-highlight-color; + } + &.active { box-shadow: 0 1px 0 rgba($ui-highlight-color, 0.3); From b383c0688a0939d25b9184e61dac72cda71f6926 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Thu, 29 Mar 2018 17:52:41 +0200 Subject: [PATCH 6/6] [Glitch] Do not overwrite some status attributes in reducer Backport 386365090c9c9063941e3f7425c2e33c7d5c5b2a --- .../flavours/glitch/reducers/statuses.js | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/app/javascript/flavours/glitch/reducers/statuses.js b/app/javascript/flavours/glitch/reducers/statuses.js index f14fcea1e8a..bc5bc180436 100644 --- a/app/javascript/flavours/glitch/reducers/statuses.js +++ b/app/javascript/flavours/glitch/reducers/statuses.js @@ -62,16 +62,20 @@ const normalizeStatus = (state, status) => { normalStatus.reblog = status.reblog.id; } - const searchContent = [status.spoiler_text, status.content].join('\n\n').replace(/
/g, '\n').replace(/<\/p>

/g, '\n\n'); + // Only calculate these values when status first encountered + // Otherwise keep the ones already in the reducer + if (!state.has(status.id)) { + const searchContent = [status.spoiler_text, status.content].join('\n\n').replace(/
/g, '\n').replace(/<\/p>

/g, '\n\n'); - const emojiMap = normalStatus.emojis.reduce((obj, emoji) => { - obj[`:${emoji.shortcode}:`] = emoji; - return obj; - }, {}); + const emojiMap = normalStatus.emojis.reduce((obj, emoji) => { + obj[`:${emoji.shortcode}:`] = emoji; + return obj; + }, {}); - normalStatus.search_index = domParser.parseFromString(searchContent, 'text/html').documentElement.textContent; - normalStatus.contentHtml = emojify(normalStatus.content, emojiMap); - normalStatus.spoilerHtml = emojify(escapeTextContentForBrowser(normalStatus.spoiler_text || ''), emojiMap); + normalStatus.search_index = domParser.parseFromString(searchContent, 'text/html').documentElement.textContent; + normalStatus.contentHtml = emojify(normalStatus.content, emojiMap); + normalStatus.spoilerHtml = emojify(escapeTextContentForBrowser(normalStatus.spoiler_text || ''), emojiMap); + } return state.update(status.id, ImmutableMap(), map => map.mergeDeep(fromJS(normalStatus))); };