From 60944d5dca0f25c668db00c0d84910f1dddc2a12 Mon Sep 17 00:00:00 2001 From: abcang Date: Wed, 13 Sep 2017 17:24:33 +0900 Subject: [PATCH] Fix height cache (#4909) --- .../mastodon/actions/height_cache.js | 17 ++++++++++ app/javascript/mastodon/actions/statuses.js | 17 ---------- .../intersection_observer_article.js | 33 ++++++++++--------- .../mastodon/components/scrollable_list.js | 17 ++++++++-- ...intersection_observer_article_container.js | 17 ++++++++++ .../mastodon/containers/status_container.js | 6 +--- app/javascript/mastodon/features/ui/index.js | 4 +-- .../mastodon/reducers/height_cache.js | 23 +++++++++++++ app/javascript/mastodon/reducers/index.js | 2 ++ app/javascript/mastodon/reducers/statuses.js | 18 ---------- 10 files changed, 93 insertions(+), 61 deletions(-) create mode 100644 app/javascript/mastodon/actions/height_cache.js create mode 100644 app/javascript/mastodon/containers/intersection_observer_article_container.js create mode 100644 app/javascript/mastodon/reducers/height_cache.js diff --git a/app/javascript/mastodon/actions/height_cache.js b/app/javascript/mastodon/actions/height_cache.js new file mode 100644 index 000000000..4c752993f --- /dev/null +++ b/app/javascript/mastodon/actions/height_cache.js @@ -0,0 +1,17 @@ +export const HEIGHT_CACHE_SET = 'HEIGHT_CACHE_SET'; +export const HEIGHT_CACHE_CLEAR = 'HEIGHT_CACHE_CLEAR'; + +export function setHeight (key, id, height) { + return { + type: HEIGHT_CACHE_SET, + key, + id, + height, + }; +}; + +export function clearHeight () { + return { + type: HEIGHT_CACHE_CLEAR, + }; +}; diff --git a/app/javascript/mastodon/actions/statuses.js b/app/javascript/mastodon/actions/statuses.js index 0b5e72c17..2204e0b14 100644 --- a/app/javascript/mastodon/actions/statuses.js +++ b/app/javascript/mastodon/actions/statuses.js @@ -23,9 +23,6 @@ export const STATUS_UNMUTE_REQUEST = 'STATUS_UNMUTE_REQUEST'; export const STATUS_UNMUTE_SUCCESS = 'STATUS_UNMUTE_SUCCESS'; export const STATUS_UNMUTE_FAIL = 'STATUS_UNMUTE_FAIL'; -export const STATUS_SET_HEIGHT = 'STATUS_SET_HEIGHT'; -export const STATUSES_CLEAR_HEIGHT = 'STATUSES_CLEAR_HEIGHT'; - export function fetchStatusRequest(id, skipLoading) { return { type: STATUS_FETCH_REQUEST, @@ -218,17 +215,3 @@ export function unmuteStatusFail(id, error) { error, }; }; - -export function setStatusHeight (id, height) { - return { - type: STATUS_SET_HEIGHT, - id, - height, - }; -}; - -export function clearStatusesHeight () { - return { - type: STATUSES_CLEAR_HEIGHT, - }; -}; diff --git a/app/javascript/mastodon/components/intersection_observer_article.js b/app/javascript/mastodon/components/intersection_observer_article.js index 347767818..bb83a4da0 100644 --- a/app/javascript/mastodon/components/intersection_observer_article.js +++ b/app/javascript/mastodon/components/intersection_observer_article.js @@ -7,10 +7,13 @@ import getRectFromEntry from '../features/ui/util/get_rect_from_entry'; export default class IntersectionObserverArticle extends ImmutablePureComponent { static propTypes = { - intersectionObserverWrapper: PropTypes.object, + intersectionObserverWrapper: PropTypes.object.isRequired, id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), index: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), listLength: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), + saveHeightKey: PropTypes.string, + cachedHeight: PropTypes.number, + onHeightChange: PropTypes.func, children: PropTypes.node, }; @@ -34,13 +37,10 @@ export default class IntersectionObserverArticle extends ImmutablePureComponent } componentDidMount () { - if (!this.props.intersectionObserverWrapper) { - // TODO: enable IntersectionObserver optimization for notification statuses. - // These are managed in notifications/index.js rather than status_list.js - return; - } - this.props.intersectionObserverWrapper.observe( - this.props.id, + const { intersectionObserverWrapper, id } = this.props; + + intersectionObserverWrapper.observe( + id, this.node, this.handleIntersection ); @@ -49,20 +49,21 @@ export default class IntersectionObserverArticle extends ImmutablePureComponent } componentWillUnmount () { - if (this.props.intersectionObserverWrapper) { - this.props.intersectionObserverWrapper.unobserve(this.props.id, this.node); - } + const { intersectionObserverWrapper, id } = this.props; + intersectionObserverWrapper.unobserve(id, this.node); this.componentMounted = false; } handleIntersection = (entry) => { + const { onHeightChange, saveHeightKey, id } = this.props; + if (this.node && this.node.children.length !== 0) { // save the height of the fully-rendered element this.height = getRectFromEntry(entry).height; - if (this.props.onHeightChange) { - this.props.onHeightChange(this.props.status, this.height); + if (onHeightChange && saveHeightKey) { + onHeightChange(saveHeightKey, id, this.height); } } @@ -94,16 +95,16 @@ export default class IntersectionObserverArticle extends ImmutablePureComponent } render () { - const { children, id, index, listLength } = this.props; + const { children, id, index, listLength, cachedHeight } = this.props; const { isIntersecting, isHidden } = this.state; - if (!isIntersecting && isHidden) { + if (!isIntersecting && (isHidden || cachedHeight)) { return (
diff --git a/app/javascript/mastodon/components/scrollable_list.js b/app/javascript/mastodon/components/scrollable_list.js index 723dd322b..3d78b3ffa 100644 --- a/app/javascript/mastodon/components/scrollable_list.js +++ b/app/javascript/mastodon/components/scrollable_list.js @@ -1,7 +1,7 @@ import React, { PureComponent } from 'react'; import { ScrollContainer } from 'react-router-scroll'; import PropTypes from 'prop-types'; -import IntersectionObserverArticle from './intersection_observer_article'; +import IntersectionObserverArticleContainer from '../containers/intersection_observer_article_container'; import LoadMore from './load_more'; import IntersectionObserverWrapper from '../features/ui/util/intersection_observer_wrapper'; import { throttle } from 'lodash'; @@ -9,6 +9,10 @@ import { List as ImmutableList } from 'immutable'; export default class ScrollableList extends PureComponent { + static contextTypes = { + router: PropTypes.object, + }; + static propTypes = { scrollKey: PropTypes.string.isRequired, onScrollToBottom: PropTypes.func, @@ -173,9 +177,16 @@ export default class ScrollableList extends PureComponent { {prepend} {React.Children.map(this.props.children, (child, index) => ( - + {child} - + ))} {loadMore} diff --git a/app/javascript/mastodon/containers/intersection_observer_article_container.js b/app/javascript/mastodon/containers/intersection_observer_article_container.js new file mode 100644 index 000000000..b6f162199 --- /dev/null +++ b/app/javascript/mastodon/containers/intersection_observer_article_container.js @@ -0,0 +1,17 @@ +import { connect } from 'react-redux'; +import IntersectionObserverArticle from '../components/intersection_observer_article'; +import { setHeight } from '../actions/height_cache'; + +const makeMapStateToProps = (state, props) => ({ + cachedHeight: state.getIn(['height_cache', props.saveHeightKey, props.id]), +}); + +const mapDispatchToProps = (dispatch) => ({ + + onHeightChange (key, id, height) { + dispatch(setHeight(key, id, height)); + }, + +}); + +export default connect(makeMapStateToProps, mapDispatchToProps)(IntersectionObserverArticle); diff --git a/app/javascript/mastodon/containers/status_container.js b/app/javascript/mastodon/containers/status_container.js index a727a87d1..c61b7d00d 100644 --- a/app/javascript/mastodon/containers/status_container.js +++ b/app/javascript/mastodon/containers/status_container.js @@ -18,7 +18,7 @@ import { blockAccount, muteAccount, } from '../actions/accounts'; -import { muteStatus, unmuteStatus, deleteStatus, setStatusHeight } from '../actions/statuses'; +import { muteStatus, unmuteStatus, deleteStatus } from '../actions/statuses'; import { initReport } from '../actions/reports'; import { openModal } from '../actions/modal'; import { defineMessages, injectIntl, FormattedMessage } from 'react-intl'; @@ -138,10 +138,6 @@ const mapDispatchToProps = (dispatch, { intl }) => ({ } }, - onHeightChange (status, height) { - dispatch(setStatusHeight(status.get('id'), height)); - }, - }); export default injectIntl(connect(makeMapStateToProps, mapDispatchToProps)(Status)); diff --git a/app/javascript/mastodon/features/ui/index.js b/app/javascript/mastodon/features/ui/index.js index 41ce0f8b5..30a52a448 100644 --- a/app/javascript/mastodon/features/ui/index.js +++ b/app/javascript/mastodon/features/ui/index.js @@ -11,7 +11,7 @@ import { debounce } from 'lodash'; import { uploadCompose } from '../../actions/compose'; import { refreshHomeTimeline } from '../../actions/timelines'; import { refreshNotifications } from '../../actions/notifications'; -import { clearStatusesHeight } from '../../actions/statuses'; +import { clearHeight } from '../../actions/height_cache'; import { WrappedSwitch, WrappedRoute } from './util/react_router_helpers'; import UploadArea from './components/upload_area'; import ColumnsAreaContainer from './containers/columns_area_container'; @@ -68,7 +68,7 @@ export default class UI extends React.PureComponent { handleResize = debounce(() => { // The cached heights are no longer accurate, invalidate - this.props.dispatch(clearStatusesHeight()); + this.props.dispatch(clearHeight()); this.setState({ width: window.innerWidth }); }, 500, { diff --git a/app/javascript/mastodon/reducers/height_cache.js b/app/javascript/mastodon/reducers/height_cache.js new file mode 100644 index 000000000..2f5716fae --- /dev/null +++ b/app/javascript/mastodon/reducers/height_cache.js @@ -0,0 +1,23 @@ +import { Map as ImmutableMap } from 'immutable'; +import { HEIGHT_CACHE_SET, HEIGHT_CACHE_CLEAR } from '../actions/height_cache'; + +const initialState = ImmutableMap(); + +const setHeight = (state, key, id, height) => { + return state.update(key, ImmutableMap(), map => map.set(id, height)); +}; + +const clearHeights = () => { + return ImmutableMap(); +}; + +export default function statuses(state = initialState, action) { + switch(action.type) { + case HEIGHT_CACHE_SET: + return setHeight(state, action.key, action.id, action.height); + case HEIGHT_CACHE_CLEAR: + return clearHeights(); + default: + return state; + } +}; diff --git a/app/javascript/mastodon/reducers/index.js b/app/javascript/mastodon/reducers/index.js index 3aaf259c2..0a8c3ce6c 100644 --- a/app/javascript/mastodon/reducers/index.js +++ b/app/javascript/mastodon/reducers/index.js @@ -19,6 +19,7 @@ import compose from './compose'; import search from './search'; import media_attachments from './media_attachments'; import notifications from './notifications'; +import height_cache from './height_cache'; const reducers = { timelines, @@ -41,6 +42,7 @@ const reducers = { search, media_attachments, notifications, + height_cache, }; export default combineReducers(reducers); diff --git a/app/javascript/mastodon/reducers/statuses.js b/app/javascript/mastodon/reducers/statuses.js index eec2a5f16..7f906bef6 100644 --- a/app/javascript/mastodon/reducers/statuses.js +++ b/app/javascript/mastodon/reducers/statuses.js @@ -15,8 +15,6 @@ import { CONTEXT_FETCH_SUCCESS, STATUS_MUTE_SUCCESS, STATUS_UNMUTE_SUCCESS, - STATUS_SET_HEIGHT, - STATUSES_CLEAR_HEIGHT, } from '../actions/statuses'; import { TIMELINE_REFRESH_SUCCESS, @@ -95,18 +93,6 @@ const filterStatuses = (state, relationship) => { return state; }; -const setHeight = (state, id, height) => { - return state.update(id, ImmutableMap(), map => map.set('height', height)); -}; - -const clearHeights = (state) => { - state.forEach(status => { - state = state.deleteIn([status.get('id'), 'height']); - }); - - return state; -}; - const initialState = ImmutableMap(); export default function statuses(state = initialState, action) { @@ -148,10 +134,6 @@ export default function statuses(state = initialState, action) { return deleteStatus(state, action.id, action.references); case ACCOUNT_BLOCK_SUCCESS: return filterStatuses(state, action.relationship); - case STATUS_SET_HEIGHT: - return setHeight(state, action.id, action.height); - case STATUSES_CLEAR_HEIGHT: - return clearHeights(state); default: return state; }