From c83beeb782bbfe0c8582798394a30c414b332882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan=20Renaudeau?= Date: Wed, 27 Jun 2018 16:34:50 +0200 Subject: [PATCH] Bugfix account.unit to refresh account page when you change it this also start splitting a bit the AccountPage component as CalculateBalance will memoize things, we want inner part to be potentially re-render if they depend on other things, we do this by connecting them to redux. redux idea is to connect leafs instead of connecting the whole tree when possible, so this fit this paradigm to me. --- .../AccountBalanceSummaryHeader.js | 125 ++++++++++++++ .../AccountPage/AccountHeaderActions.js | 100 +++++++++++ src/components/AccountPage/index.js | 160 +++--------------- src/components/BalanceSummary/index.js | 1 + src/components/CalculateBalance.js | 10 +- 5 files changed, 259 insertions(+), 137 deletions(-) create mode 100644 src/components/AccountPage/AccountBalanceSummaryHeader.js create mode 100644 src/components/AccountPage/AccountHeaderActions.js diff --git a/src/components/AccountPage/AccountBalanceSummaryHeader.js b/src/components/AccountPage/AccountBalanceSummaryHeader.js new file mode 100644 index 00000000..b396e801 --- /dev/null +++ b/src/components/AccountPage/AccountBalanceSummaryHeader.js @@ -0,0 +1,125 @@ +// @flow + +import React, { PureComponent } from 'react' +import { createStructuredSelector } from 'reselect' +import { compose } from 'redux' +import { connect } from 'react-redux' +import { translate } from 'react-i18next' +import type { Currency, Account } from '@ledgerhq/live-common/lib/types' + +import type { T } from 'types/common' + +import { saveSettings } from 'actions/settings' +import { accountSelector } from 'reducers/accounts' +import { counterValueCurrencySelector, selectedTimeRangeSelector } from 'reducers/settings' +import type { TimeRange } from 'reducers/settings' + +import { + BalanceTotal, + BalanceSinceDiff, + BalanceSincePercent, +} from 'components/BalanceSummary/BalanceInfos' +import Box from 'components/base/Box' +import FormattedVal from 'components/base/FormattedVal' +import PillsDaysCount from 'components/PillsDaysCount' + +type OwnProps = { + isAvailable: boolean, + totalBalance: number, + sinceBalance: number, + refBalance: number, + accountId: string, // eslint-disable-line +} + +type Props = OwnProps & { + counterValue: Currency, + t: T, + account: Account, + saveSettings: ({ selectedTimeRange: TimeRange }) => *, + selectedTimeRange: TimeRange, +} + +const mapStateToProps = createStructuredSelector({ + account: accountSelector, + counterValue: counterValueCurrencySelector, + selectedTimeRange: selectedTimeRangeSelector, +}) + +const mapDispatchToProps = { + saveSettings, +} + +class AccountBalanceSummaryHeader extends PureComponent { + handleChangeSelectedTime = item => { + this.props.saveSettings({ selectedTimeRange: item.key }) + } + + render() { + const { + account, + t, + counterValue, + selectedTimeRange, + isAvailable, + totalBalance, + sinceBalance, + refBalance, + } = this.props + + return ( + + + + + + + + + + + + + + + ) + } +} + +export default compose( + connect( + mapStateToProps, + mapDispatchToProps, + ), + translate(), // FIXME t() is not even needed directly here. should be underlying component responsability to inject it +)(AccountBalanceSummaryHeader) diff --git a/src/components/AccountPage/AccountHeaderActions.js b/src/components/AccountPage/AccountHeaderActions.js new file mode 100644 index 00000000..eb60494b --- /dev/null +++ b/src/components/AccountPage/AccountHeaderActions.js @@ -0,0 +1,100 @@ +// @flow + +import React, { PureComponent, Fragment } from 'react' +import { compose } from 'redux' +import { connect } from 'react-redux' +import { translate } from 'react-i18next' +import styled from 'styled-components' +import type { Account } from '@ledgerhq/live-common/lib/types' +import Tooltip from 'components/base/Tooltip' + +import { MODAL_SEND, MODAL_RECEIVE, MODAL_SETTINGS_ACCOUNT } from 'config/constants' + +import type { T } from 'types/common' + +import { rgba } from 'styles/helpers' + +import { openModal } from 'reducers/modals' + +import IconAccountSettings from 'icons/AccountSettings' +import IconReceive from 'icons/Receive' +import IconSend from 'icons/Send' + +import Box, { Tabbable } from 'components/base/Box' +import Button from 'components/base/Button' + +const ButtonSettings = styled(Tabbable).attrs({ + cursor: 'pointer', + align: 'center', + justify: 'center', + borderRadius: 1, +})` + width: 40px; + height: 40px; + + &:hover { + color: ${p => (p.disabled ? '' : p.theme.colors.dark)}; + background: ${p => (p.disabled ? '' : rgba(p.theme.colors.fog, 0.2))}; + } + + &:active { + background: ${p => (p.disabled ? '' : rgba(p.theme.colors.fog, 0.3))}; + } +` + +const mapStateToProps = null + +const mapDispatchToProps = { + openModal, +} + +type OwnProps = { + account: Account, +} + +type Props = OwnProps & { + t: T, + openModal: Function, +} + +class AccountHeaderActions extends PureComponent { + render() { + const { account, openModal, t } = this.props + return ( + + {account.operations.length > 0 && ( + + + + + + )} + t('app:account.settings.title')}> + openModal(MODAL_SETTINGS_ACCOUNT, { account })}> + + + + + + + ) + } +} + +export default compose( + connect( + mapStateToProps, + mapDispatchToProps, + ), + translate(), +)(AccountHeaderActions) diff --git a/src/components/AccountPage/index.js b/src/components/AccountPage/index.js index 730ba135..397c6f4e 100644 --- a/src/components/AccountPage/index.js +++ b/src/components/AccountPage/index.js @@ -5,19 +5,8 @@ import { compose } from 'redux' import { connect } from 'react-redux' import { translate } from 'react-i18next' import { Redirect } from 'react-router' -import styled from 'styled-components' import type { Currency, Account } from '@ledgerhq/live-common/lib/types' -import SyncOneAccountOnMount from 'components/SyncOneAccountOnMount' -import Tooltip from 'components/base/Tooltip' -import TrackPage from 'analytics/TrackPage' - -import { MODAL_SEND, MODAL_RECEIVE, MODAL_SETTINGS_ACCOUNT } from 'config/constants' - import type { T } from 'types/common' - -import { rgba } from 'styles/helpers' - -import { saveSettings } from 'actions/settings' import { accountSelector } from 'reducers/accounts' import { counterValueCurrencySelector, @@ -26,47 +15,19 @@ import { timeRangeDaysByKey, } from 'reducers/settings' import type { TimeRange } from 'reducers/settings' -import { openModal } from 'reducers/modals' - -import IconAccountSettings from 'icons/AccountSettings' -import IconReceive from 'icons/Receive' -import IconSend from 'icons/Send' +import TrackPage from 'analytics/TrackPage' +import SyncOneAccountOnMount from 'components/SyncOneAccountOnMount' import BalanceSummary from 'components/BalanceSummary' -import { - BalanceTotal, - BalanceSinceDiff, - BalanceSincePercent, -} from 'components/BalanceSummary/BalanceInfos' -import Box, { Tabbable } from 'components/base/Box' -import Button from 'components/base/Button' -import FormattedVal from 'components/base/FormattedVal' -import PillsDaysCount from 'components/PillsDaysCount' +import Box from 'components/base/Box' import OperationsList from 'components/OperationsList' import StickyBackToTop from 'components/StickyBackToTop' import AccountHeader from './AccountHeader' +import AccountHeaderActions from './AccountHeaderActions' +import AccountBalanceSummaryHeader from './AccountBalanceSummaryHeader' import EmptyStateAccount from './EmptyStateAccount' -const ButtonSettings = styled(Tabbable).attrs({ - cursor: 'pointer', - align: 'center', - justify: 'center', - borderRadius: 1, -})` - width: 40px; - height: 40px; - - &:hover { - color: ${p => (p.disabled ? '' : p.theme.colors.dark)}; - background: ${p => (p.disabled ? '' : rgba(p.theme.colors.fog, 0.2))}; - } - - &:active { - background: ${p => (p.disabled ? '' : rgba(p.theme.colors.fog, 0.3))}; - } -` - const mapStateToProps = (state, props) => ({ account: accountSelector(state, { accountId: props.match.params.id }), counterValue: counterValueCurrencySelector(state), @@ -74,74 +35,54 @@ const mapStateToProps = (state, props) => ({ selectedTimeRange: selectedTimeRangeSelector(state), }) -const mapDispatchToProps = { - openModal, - saveSettings, -} +const mapDispatchToProps = null type Props = { counterValue: Currency, t: T, account?: Account, - openModal: Function, - saveSettings: ({ selectedTimeRange: TimeRange }) => *, selectedTimeRange: TimeRange, } class AccountPage extends PureComponent { - handleChangeSelectedTime = item => { - this.props.saveSettings({ selectedTimeRange: item.key }) + renderBalanceSummaryHeader = ({ isAvailable, totalBalance, sinceBalance, refBalance }) => { + const { account } = this.props + if (!account) return null + return ( + + ) } - _cacheBalance = null - render() { - const { account, openModal, t, counterValue, selectedTimeRange } = this.props + const { account, t, counterValue, selectedTimeRange } = this.props const daysCount = timeRangeDaysByKey[selectedTimeRange] - // Don't even throw if we jumped in wrong account route if (!account) { return } return ( - // Force re-render account page, for avoid animation + // `key` forces re-render account page when going an another account (skip animations) + + - - {account.operations.length > 0 && ( - - - - - - )} - t('app:account.settings.title')}> - openModal(MODAL_SETTINGS_ACCOUNT, { account })}> - - - - - - + + {account.operations.length > 0 ? ( @@ -152,59 +93,12 @@ class AccountPage extends PureComponent { counterValue={counterValue} daysCount={daysCount} selectedTimeRange={selectedTimeRange} - renderHeader={({ isAvailable, totalBalance, sinceBalance, refBalance }) => ( - - - - - - - - - - - - - - - )} + renderHeader={this.renderBalanceSummaryHeader} /> + + ) : ( diff --git a/src/components/BalanceSummary/index.js b/src/components/BalanceSummary/index.js index a517c3ad..e0b9c02e 100644 --- a/src/components/BalanceSummary/index.js +++ b/src/components/BalanceSummary/index.js @@ -35,6 +35,7 @@ const BalanceSummary = ({ selectedTimeRange, }: Props) => { const account = accounts.length === 1 ? accounts[0] : undefined + // FIXME This nesting 😱 return ( diff --git a/src/components/CalculateBalance.js b/src/components/CalculateBalance.js index d94f0b6e..532372e8 100644 --- a/src/components/CalculateBalance.js +++ b/src/components/CalculateBalance.js @@ -32,6 +32,7 @@ type Props = OwnProps & { balanceStart: number, balanceEnd: number, isAvailable: boolean, + hash: string, } const mapStateToProps = (state: State, props: OwnProps) => { @@ -71,19 +72,20 @@ const mapStateToProps = (state: State, props: OwnProps) => { ({ ...item, originalValue: originalValues[i] || 0 }), ) + const balanceEnd = balanceHistory[balanceHistory.length - 1].value + return { isAvailable, balanceHistory, balanceStart: balanceHistory[0].value, - balanceEnd: balanceHistory[balanceHistory.length - 1].value, + balanceEnd, + hash: `${balanceHistory.length}_${balanceEnd}`, } } -const hash = ({ balanceHistory, balanceEnd }) => `${balanceHistory.length}_${balanceEnd}` - class CalculateBalance extends Component { shouldComponentUpdate(nextProps) { - return hash(nextProps) !== hash(this.props) + return nextProps.hash !== this.props.hash } render() { const { children } = this.props