Refactor the infamous three-valued boolean into two booleans, trying to simplify the logic

This commit is contained in:
Thibaut Girka 2018-03-28 15:40:34 +02:00
parent 259bc9840b
commit 2888f74c12
5 changed files with 49 additions and 41 deletions

View file

@ -50,7 +50,8 @@ export default class Status extends ImmutablePureComponent {
}; };
state = { state = {
isExpanded: null, isExpanded: false,
isCollapsed: false,
autoCollapsed: false, autoCollapsed: false,
} }
@ -71,6 +72,7 @@ export default class Status extends ImmutablePureComponent {
updateOnStates = [ updateOnStates = [
'isExpanded', 'isExpanded',
'isCollapsed',
] ]
// If our settings have changed to disable collapsed statuses, then we // 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. // uncollapse our status accordingly.
componentWillReceiveProps (nextProps) { componentWillReceiveProps (nextProps) {
if (!nextProps.settings.getIn(['collapsed', 'enabled'])) { if (!nextProps.settings.getIn(['collapsed', 'enabled'])) {
if (this.state.isExpanded === false) { if (this.state.isCollapsed) {
this.setExpansion(null); this.setCollapsed(false);
} }
} else if ( } else if (
nextProps.collapse !== this.props.collapse && nextProps.collapse !== this.props.collapse &&
nextProps.collapse !== undefined 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, // 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 // 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. // account.
// The cases where a status should be collapsed are: // The cases where a status should be collapsed are:
@ -138,7 +140,7 @@ export default class Status extends ImmutablePureComponent {
return false; return false;
} }
}()) { }()) {
this.setExpansion(false); this.setCollapsed(true);
// Hack to fix timeline jumps on second rendering when auto-collapsing // Hack to fix timeline jumps on second rendering when auto-collapsing
this.setState({ autoCollapsed: true }); 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 // `setCollapsed()` sets the value of `isCollapsed` in our state, that is,
// one argument, `value`, which gives the desired value for `isExpanded`. // whether the toot is collapsed or not.
// The default for this argument is `null`.
// `setExpansion()` automatically checks for us whether toot collapsing // `setCollapsed()` automatically checks for us whether toot collapsing
// is enabled, so we don't have to. // 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) => { setExpansion = (value) => {
switch (true) { this.setState({ isExpanded: value });
case value === undefined || value === null: if (value) {
this.setState({ isExpanded: null }); this.setCollapsed(false);
break;
case !value && this.props.settings.getIn(['collapsed', 'enabled']):
this.setState({ isExpanded: false });
break;
case !!value:
this.setState({ isExpanded: true });
break;
} }
} }
@ -192,7 +197,7 @@ export default class Status extends ImmutablePureComponent {
parseClick = (e, destination) => { parseClick = (e, destination) => {
const { router } = this.context; const { router } = this.context;
const { status } = this.props; const { status } = this.props;
const { isExpanded } = this.state; const { isCollapsed } = this.state;
if (!router) return; if (!router) return;
if (destination === undefined) { if (destination === undefined) {
destination = `/statuses/${ destination = `/statuses/${
@ -200,9 +205,9 @@ export default class Status extends ImmutablePureComponent {
}`; }`;
} }
if (e.button === 0) { if (e.button === 0) {
if (isExpanded === false) this.setExpansion(null); if (isCollapsed) this.setCollapsed(false);
else if (e.shiftKey) { else if (e.shiftKey) {
this.setExpansion(false); this.setCollapsed(true);
document.getSelection().removeAllRanges(); document.getSelection().removeAllRanges();
} else router.history.push(destination); } else router.history.push(destination);
e.preventDefault(); e.preventDefault();
@ -219,7 +224,7 @@ export default class Status extends ImmutablePureComponent {
handleExpandedToggle = () => { handleExpandedToggle = () => {
if (this.props.status.get('spoiler_text')) { 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, handleRef,
parseClick, parseClick,
setExpansion, setExpansion,
setCollapsed,
} = this; } = this;
const { router } = this.context; const { router } = this.context;
const { const {
@ -294,7 +300,7 @@ export default class Status extends ImmutablePureComponent {
hidden, hidden,
...other ...other
} = this.props; } = this.props;
const { isExpanded } = this.state; const { isExpanded, isCollapsed } = this.state;
let background = null; let background = null;
let attachments = null; let attachments = null;
let media = null; let media = null;
@ -413,8 +419,8 @@ export default class Status extends ImmutablePureComponent {
}; };
const computedClass = classNames('status', `status-${status.get('visibility')}`, { const computedClass = classNames('status', `status-${status.get('visibility')}`, {
collapsed: isExpanded === false, collapsed: isCollapsed,
'has-background': isExpanded === false && background, 'has-background': isCollapsed && background,
muted, muted,
}, 'focusable'); }, 'focusable');
@ -422,7 +428,7 @@ export default class Status extends ImmutablePureComponent {
<HotKeys handlers={handlers}> <HotKeys handlers={handlers}>
<div <div
className={computedClass} className={computedClass}
style={isExpanded === false && background ? { backgroundImage: `url(${background})` } : null} style={isCollapsed && background ? { backgroundImage: `url(${background})` } : null}
{...selectorAttribs} {...selectorAttribs}
ref={handleRef} ref={handleRef}
tabIndex='0' tabIndex='0'
@ -437,11 +443,11 @@ export default class Status extends ImmutablePureComponent {
notificationId={this.props.notificationId} notificationId={this.props.notificationId}
/> />
) : null} ) : null}
{!muted || isExpanded !== false ? ( {!muted || !isCollapsed ? (
<StatusHeader <StatusHeader
status={status} status={status}
friend={account} friend={account}
collapsed={isExpanded === false} collapsed={isCollapsed}
parseClick={parseClick} parseClick={parseClick}
/> />
) : null} ) : null}
@ -450,8 +456,8 @@ export default class Status extends ImmutablePureComponent {
status={status} status={status}
mediaIcon={mediaIcon} mediaIcon={mediaIcon}
collapsible={settings.getIn(['collapsed', 'enabled'])} collapsible={settings.getIn(['collapsed', 'enabled'])}
collapsed={isExpanded === false} collapsed={isCollapsed}
setExpansion={setExpansion} setCollapsed={setCollapsed}
/> />
</header> </header>
<StatusContent <StatusContent
@ -463,7 +469,7 @@ export default class Status extends ImmutablePureComponent {
parseClick={parseClick} parseClick={parseClick}
disabled={!router} disabled={!router}
/> />
{isExpanded !== false || !muted ? ( {!isCollapsed || !muted ? (
<StatusActionBar <StatusActionBar
{...other} {...other}
status={status} status={status}

View file

@ -11,6 +11,7 @@ export default class StatusContent extends React.PureComponent {
static propTypes = { static propTypes = {
status: ImmutablePropTypes.map.isRequired, status: ImmutablePropTypes.map.isRequired,
expanded: PropTypes.bool, expanded: PropTypes.bool,
collapsed: PropTypes.bool,
setExpansion: PropTypes.func, setExpansion: PropTypes.func,
media: PropTypes.element, media: PropTypes.element,
mediaIcon: PropTypes.string, mediaIcon: PropTypes.string,
@ -64,7 +65,7 @@ export default class StatusContent extends React.PureComponent {
} }
onLinkClick = (e) => { onLinkClick = (e) => {
if (this.props.expanded === false) { if (this.props.collapsed) {
if (this.props.parseClick) this.props.parseClick(e); if (this.props.parseClick) this.props.parseClick(e);
} }
} }
@ -112,7 +113,7 @@ export default class StatusContent extends React.PureComponent {
e.preventDefault(); e.preventDefault();
if (this.props.setExpansion) { if (this.props.setExpansion) {
this.props.setExpansion(this.props.expanded ? null : true); this.props.setExpansion(!this.props.expanded);
} else { } else {
this.setState({ hidden: !this.state.hidden }); this.setState({ hidden: !this.state.hidden });
} }

View file

@ -22,15 +22,15 @@ export default class StatusIcons extends React.PureComponent {
mediaIcon: PropTypes.string, mediaIcon: PropTypes.string,
collapsible: PropTypes.bool, collapsible: PropTypes.bool,
collapsed: PropTypes.bool, collapsed: PropTypes.bool,
setExpansion: PropTypes.func.isRequired, setCollapsed: PropTypes.func.isRequired,
intl: PropTypes.object.isRequired, intl: PropTypes.object.isRequired,
}; };
// Handles clicks on collapsed button // Handles clicks on collapsed button
handleCollapsedClick = (e) => { handleCollapsedClick = (e) => {
const { collapsed, setExpansion } = this.props; const { collapsed, setCollapsed } = this.props;
if (e.button === 0) { if (e.button === 0) {
setExpansion(collapsed ? null : false); setCollapsed(!collapsed);
e.preventDefault(); e.preventDefault();
} }
} }

View file

@ -114,6 +114,7 @@ export default class DetailedStatus extends ImmutablePureComponent {
media={media} media={media}
mediaIcon={mediaIcon} mediaIcon={mediaIcon}
expanded={expanded} expanded={expanded}
collapsed={false}
setExpansion={setExpansion} setExpansion={setExpansion}
/> />

View file

@ -76,7 +76,7 @@ export default class Status extends ImmutablePureComponent {
state = { state = {
fullscreen: false, fullscreen: false,
isExpanded: null, isExpanded: false,
}; };
componentWillMount () { componentWillMount () {
@ -96,7 +96,7 @@ export default class Status extends ImmutablePureComponent {
handleExpandedToggle = () => { handleExpandedToggle = () => {
if (this.props.status.get('spoiler_text')) { 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 => { setExpansion = value => {
this.setState({ isExpanded: value ? true : null }); this.setState({ isExpanded: value });
} }
setRef = c => { setRef = c => {