From 58168acd044db07211f95aa1c041667cf9cf643a Mon Sep 17 00:00:00 2001 From: Jen Jones Arnesen Date: Wed, 18 Mar 2020 13:50:52 -0700 Subject: [PATCH 1/2] fix: calculate item height accounting for long title DHIS2-8421 --- src/components/Item/ItemHeader.js | 21 ++++++++++- src/components/Item/VisualizationItem/Item.js | 37 ++++++++++--------- .../VisualizationItem/__tests__/Item.spec.js | 6 +++ .../__tests__/__snapshots__/Item.spec.js.snap | 26 +++++++++---- 4 files changed, 63 insertions(+), 27 deletions(-) diff --git a/src/components/Item/ItemHeader.js b/src/components/Item/ItemHeader.js index a35a217ed..786f6632e 100644 --- a/src/components/Item/ItemHeader.js +++ b/src/components/Item/ItemHeader.js @@ -8,6 +8,9 @@ import DeleteItemButton from './DeleteItemButton'; import classes from './styles/ItemHeader.module.css'; +// This is the margin-top + margin-bottom defined in the css file +export const HEADER_MARGIN_HEIGHT = 12; + const ItemHeader = props => { const { title, @@ -15,12 +18,13 @@ const ItemHeader = props => { actionButtons, itemId, acRemoveDashboardItem, + forwardedRef, } = props; const handleDeleteItem = () => acRemoveDashboardItem(itemId); return ( -
+

{title}

{editMode ? (
@@ -41,10 +45,15 @@ ItemHeader.propTypes = { acRemoveDashboardItem: PropTypes.func, actionButtons: PropTypes.node, editMode: PropTypes.bool, + forwardedRef: PropTypes.object, itemId: PropTypes.string, title: PropTypes.string, }; +ItemHeader.defaultProps = { + forwardedRef: {}, +}; + const mapStateToProps = state => ({ editMode: sGetIsEditing(state), }); @@ -53,7 +62,15 @@ const mapDispatchToProps = { acRemoveDashboardItem, }; -export default connect( +const ConnectedItemHeader = connect( mapStateToProps, mapDispatchToProps )(ItemHeader); + +// TODO this is a false positive that is fixed in eslint-plugin-react v7.15 +// github.com/yannickcr/eslint-plugin-react/blob/master/CHANGELOG.md +/* eslint-disable react/display-name */ +export default React.forwardRef((props, ref) => ( + +)); +/* eslint-enable react/display-name */ diff --git a/src/components/Item/VisualizationItem/Item.js b/src/components/Item/VisualizationItem/Item.js index 8a3c20159..79352f3d6 100644 --- a/src/components/Item/VisualizationItem/Item.js +++ b/src/components/Item/VisualizationItem/Item.js @@ -8,7 +8,7 @@ import i18n from '@dhis2/d2-i18n'; import DefaultPlugin from './DefaultPlugin'; import FatalErrorBoundary from './FatalErrorBoundary'; -import ItemHeader from '../ItemHeader'; +import ItemHeader, { HEADER_MARGIN_HEIGHT } from '../ItemHeader'; import ItemHeaderButtons from './ItemHeaderButtons'; import ItemFooter from './ItemFooter'; import * as pluginManager from './plugin'; @@ -30,8 +30,6 @@ import { colors } from '@dhis2/ui-core'; import { getVisualizationConfig } from './plugin'; import LoadingMask from './LoadingMask'; -const HEADER_HEIGHT = 45; - const styles = { icon: { width: 16, @@ -76,12 +74,15 @@ export class Item extends Component { this.d2 = context.d2; this.contentRef = React.createRef(); + this.headerRef = React.createRef(); this.memoizedApplyFilters = memoizeOne(this.applyFilters); this.memoizedGetVisualizationConfig = memoizeOne( getVisualizationConfig ); + + this.memoizedGetContentStyle = memoizeOne(this.getContentStyle); } async componentDidMount() { @@ -171,7 +172,11 @@ export class Item extends Component { const props = { ...this.props, visualization, - style: this.getContentStyle(), + style: this.memoizedGetContentStyle( + this.props.item.originalHeight, + this.headerRef.current.clientHeight, + this.contentRef ? this.contentRef.offsetHeight : null + ), }; switch (activeType) { @@ -275,19 +280,16 @@ export class Item extends Component { this.props.visualization ); - getContentStyle = () => { - const { item, editMode } = this.props; - const PADDING_BOTTOM = 4; - - return !editMode - ? { - height: item.originalHeight - HEADER_HEIGHT - PADDING_BOTTOM, - } - : { - height: this.contentRef - ? this.contentRef.offsetHeight - : item.originalHeight - HEADER_HEIGHT - PADDING_BOTTOM, - }; + getContentStyle = (originalHeight, headerHeight, offsetHeight) => { + const height = originalHeight - headerHeight - HEADER_MARGIN_HEIGHT; + + if (!this.props.editMode) { + return { height }; + } + + return { + height: offsetHeight || height, + }; }; render() { @@ -312,6 +314,7 @@ export class Item extends Component { title={pluginManager.getName(item)} itemId={item.id} actionButtons={actionButtons} + ref={this.headerRef} />
{ }; }); +const mockHeaderRef = { clientHeight: 50 }; + describe('VisualizationItem/Item', () => { let props; let shallowItem; @@ -75,6 +77,8 @@ describe('VisualizationItem/Item', () => { const component = canvas(); + component.instance().headerRef.current = mockHeaderRef; + component.setState({ configLoaded: true }); const visPlugin = component.find(VisualizationPlugin); @@ -101,6 +105,7 @@ describe('VisualizationItem/Item', () => { }; const component = canvas(); + component.instance().headerRef.current = mockHeaderRef; component.setState({ configLoaded: true }); @@ -129,6 +134,7 @@ describe('VisualizationItem/Item', () => { expect(canvas()).toMatchSnapshot(); const component = canvas(); + component.instance().headerRef.current = mockHeaderRef; component.setState({ configLoaded: true }); diff --git a/src/components/Item/VisualizationItem/__tests__/__snapshots__/Item.spec.js.snap b/src/components/Item/VisualizationItem/__tests__/__snapshots__/Item.spec.js.snap index db647a5bb..a87f2708a 100644 --- a/src/components/Item/VisualizationItem/__tests__/__snapshots__/Item.spec.js.snap +++ b/src/components/Item/VisualizationItem/__tests__/__snapshots__/Item.spec.js.snap @@ -53,7 +53,7 @@ ShallowWrapper { "nodeType": "function", "props": Object { "children": Array [ - Date: Wed, 18 Mar 2020 14:11:27 -0700 Subject: [PATCH 2/2] fix: do same thing for both view and edit mode --- src/components/Item/VisualizationItem/Item.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/components/Item/VisualizationItem/Item.js b/src/components/Item/VisualizationItem/Item.js index 79352f3d6..d94043a90 100644 --- a/src/components/Item/VisualizationItem/Item.js +++ b/src/components/Item/VisualizationItem/Item.js @@ -280,15 +280,12 @@ export class Item extends Component { this.props.visualization ); - getContentStyle = (originalHeight, headerHeight, offsetHeight) => { - const height = originalHeight - headerHeight - HEADER_MARGIN_HEIGHT; - - if (!this.props.editMode) { - return { height }; - } + getContentStyle = (originalHeight, headerHeight, measuredHeight) => { + const calculatedHeight = + originalHeight - headerHeight - HEADER_MARGIN_HEIGHT; return { - height: offsetHeight || height, + height: measuredHeight || calculatedHeight, }; };