fix: phone landscape scrolling in edit mode was not available (#1594)

Code changes to fix: * Edit controlbar now scrolls away same as View control bar (added overflow-y: auto) * add dashboard-scroll-container class bc progressive loading listens to the scrolling to determine when to load an item. * remove bottom padding on the NoContentMessage since it sometimes caused the content of the scroll container to exceed the container height, forcing a scrollbar even though there was no real scroll content.
dhis2 · Mar 3, 2021 · 6bfc650 · 6bfc650
1 parent 56ad5b5
commit 6bfc650
Show file tree

Hide file tree

Showing 16 changed files with 125 additions and 22 deletions.
diff --git a/cypress/integration/ui/responsive_dashboard.feature b/cypress/integration/ui/responsive_dashboard.feature
@@ -53,3 +53,22 @@ Feature: Small screen dashboard
         When I go to small screen
         And I click on the "Period" filter badge
         Then the filter modal is not opened
+
+    @nonmutating
+    Scenario: Dashboards bar scrolls away in phone landscape
+        Given I open the "Delivery" dashboard
+        When I go to phone landscape
+        And I scroll down
+        Then the dashboards bar is not visible
+        When I scroll to top
+        Then the dashboards bar is visible
+
+    @nonmutating
+    Scenario: Edit bar scrolls away in phone landscape
+        Given I open the "Delivery" dashboard
+        When I choose to edit dashboard
+        And I go to phone landscape
+        And I scroll down
+        Then the edit control bar is not visible
+        When I scroll to top
+        Then the edit control bar is visible
diff --git a/cypress/integration/ui/responsive_dashboard/phone_landscape.js b/cypress/integration/ui/responsive_dashboard/phone_landscape.js
@@ -0,0 +1,42 @@
+import { When, Then } from 'cypress-cucumber-preprocessor/steps'
+import {
+    dashboardsBarSel,
+    outerScrollContainerSel,
+    editControlBarSel,
+} from '../../../selectors/viewDashboard'
+
+// Scenario: Dashboards bar scrolls away in phone landscape
+
+When('I go to phone landscape', () => {
+    cy.viewport(600, 480)
+    // to account for debounced window resize
+    cy.wait(100) // eslint-disable-line cypress/no-unnecessary-waiting
+})
+
+When('I scroll down', () => {
+    cy.get(outerScrollContainerSel).scrollTo('bottom')
+    // this item is on the bottom of the Delivery dashboard
+    cy.contains(
+        'Births attended by skilled health personnel by orgunit last year'
+    ).should('be.visible')
+})
+
+Then('the dashboards bar is not visible', () => {
+    cy.get(dashboardsBarSel).should('not.be.visible')
+})
+
+When('I scroll to top', () => {
+    cy.get(outerScrollContainerSel).scrollTo('top')
+})
+
+Then('the dashboards bar is visible', () => {
+    cy.get(dashboardsBarSel).should('be.visible')
+})
+
+Then('the edit control bar is not visible', () => {
+    cy.get(editControlBarSel).should('not.be.visible')
+})
+
+Then('the edit control bar is visible', () => {
+    cy.get(editControlBarSel).should('be.visible')
+})
diff --git a/cypress/selectors/viewDashboard.js b/cypress/selectors/viewDashboard.js
@@ -14,3 +14,7 @@ export const dashboardUnstarredSel = '[data-test="dashboard-unstarred"]'
 
 export const dragHandleSel = '[data-test="controlbar-drag-handle"]'
 export const dashboardsBarSel = '[data-test="dashboards-bar"]'
+
+export const outerScrollContainerSel = '[data-test="outer-scroll-container"]'
+
+export const editControlBarSel = '[data-test="edit-control-bar"]'
diff --git a/cypress/support/index.js b/cypress/support/index.js
@@ -8,3 +8,11 @@ if (!isStubMode(getDefaultMode())) {
     // log in if using a live backend
     enableAutoLogin()
 }
+
+const resizeObserverLoopErrRe = /^[^(ResizeObserver loop limit exceeded)]/
+Cypress.on('uncaught:exception', err => {
+    /* returning false here prevents Cypress from failing the test */
+    if (resizeObserverLoopErrRe.test(err.message)) {
+        return false
+    }
+})
diff --git a/src/components/ControlBar/EditBar.js b/src/components/ControlBar/EditBar.js
@@ -216,7 +216,7 @@ const EditBar = props => {
 
     return (
         <>
-            <div className={classes.editBar}>
+            <div className={classes.editBar} data-test="edit-control-bar">
                 <div className={classes.controls}>
                     {props.updateAccess ? renderActionButtons() : null}
                     <Button secondary onClick={onDiscard}>

diff --git a/src/components/ControlBar/__tests__/__snapshots__/EditBar.spec.js.snap b/src/components/ControlBar/__tests__/__snapshots__/EditBar.spec.js.snap
@@ -4,6 +4,7 @@ exports[`renders Save and Discard buttons but no dialogs when no dashboard id 1`
 <div>
   <div
     class="editBar"
+    data-test="edit-control-bar"
   >
     <div
       class="controls"
@@ -68,6 +69,7 @@ exports[`renders Translate, Delete, and Discard buttons when delete access 1`] =
 <div>
   <div
     class="editBar"
+    data-test="edit-control-bar"
   >
     <div
       class="controls"
@@ -148,6 +150,7 @@ exports[`renders only the Go to Dashboards button when no update access 1`] = `
 <div>
   <div
     class="editBar"
+    data-test="edit-control-bar"
   >
     <div
       class="controls"
@@ -168,6 +171,7 @@ exports[`renders the EditBar 1`] = `
 <div>
   <div
     class="editBar"
+    data-test="edit-control-bar"
   >
     <div
       class="controls"
@@ -240,6 +244,7 @@ exports[`shows the confirm delete dialog when delete button clicked 1`] = `
 <DocumentFragment>
   <div
     class="editBar"
+    data-test="edit-control-bar"
   >
     <div
       class="controls"
@@ -323,6 +328,7 @@ exports[`shows the translate dialog 1`] = `
 <DocumentFragment>
   <div
     class="editBar"
+    data-test="edit-control-bar"
   >
     <div
       class="controls"

diff --git a/src/components/Dashboard/EditDashboard.js b/src/components/Dashboard/EditDashboard.js
@@ -2,6 +2,7 @@ import React, { useEffect } from 'react'
 import { connect } from 'react-redux'
 import i18n from '@dhis2/d2-i18n'
 import PropTypes from 'prop-types'
+import cx from 'classnames'
 
 import DashboardContainer from './DashboardContainer'
 import EditTitleBar from '../TitleBar/EditTitleBar'
@@ -41,7 +42,10 @@ const EditDashboard = props => {
 
     return (
         <>
-            <div className={classes.container}>
+            <div
+                className={cx(classes.container, 'dashboard-scroll-container')}
+                data-test="outer-scroll-container"
+            >
                 <EditBar />
                 {props.updateAccess ? (
                     renderGrid()

diff --git a/src/components/Dashboard/NewDashboard.js b/src/components/Dashboard/NewDashboard.js
@@ -2,6 +2,7 @@ import React, { useEffect } from 'react'
 import { connect } from 'react-redux'
 import PropTypes from 'prop-types'
 import i18n from '@dhis2/d2-i18n'
+import cx from 'classnames'
 
 import DashboardContainer from './DashboardContainer'
 import EditBar from '../ControlBar/EditBar'
@@ -22,7 +23,9 @@ const NewDashboard = props => {
 
     return (
         <>
-            <div className={classes.container}>
+            <div
+                className={cx(classes.container, 'dashboard-scroll-container')}
+            >
                 <EditBar />
                 {props.isPrintPreviewView ? (
                     <LayoutPrintPreview fromEdit={true} />

diff --git a/src/components/Dashboard/ViewDashboard.js b/src/components/Dashboard/ViewDashboard.js
@@ -52,7 +52,10 @@ export const ViewDashboard = props => {
     const onExpandedChanged = expanded => setControlbarExpanded(expanded)
 
     return (
-        <div className={cx(classes.container, 'dashboard-scroll-container')}>
+        <div
+            className={cx(classes.container, 'dashboard-scroll-container')}
+            data-test="outer-scroll-container"
+        >
             <DashboardsBar
                 expanded={controlbarExpanded}
                 onExpandedChanged={onExpandedChanged}

diff --git a/src/components/Dashboard/__tests__/__snapshots__/EditDashboard.spec.js.snap b/src/components/Dashboard/__tests__/__snapshots__/EditDashboard.spec.js.snap
@@ -3,7 +3,8 @@
 exports[`EditDashboard renders dashboard 1`] = `
 <div>
   <div
-    class="container"
+    class="container dashboard-scroll-container"
+    data-test="outer-scroll-container"
   >
     <div>
       EditBar
@@ -77,13 +78,14 @@ exports[`EditDashboard renders dashboard 1`] = `
 exports[`EditDashboard renders message when not enough access 1`] = `
 <div>
   <div
-    class="container"
+    class="container dashboard-scroll-container"
+    data-test="outer-scroll-container"
   >
     <div>
       EditBar
     </div>
     <div
-      style="padding: 50px 10px; text-align: center; font-size: 15px; font-weight: 500; color: rgb(110, 122, 138);"
+      class="container"
     >
       No access
     </div>
@@ -146,7 +148,8 @@ exports[`EditDashboard renders message when not enough access 1`] = `
 exports[`EditDashboard renders print preview 1`] = `
 <div>
   <div
-    class="container"
+    class="container dashboard-scroll-container"
+    data-test="outer-scroll-container"
   >
     <div>
       EditBar

diff --git a/src/components/Dashboard/__tests__/__snapshots__/NewDashboard.spec.js.snap b/src/components/Dashboard/__tests__/__snapshots__/NewDashboard.spec.js.snap
@@ -3,7 +3,7 @@
 exports[`NewDashboard renders dashboard 1`] = `
 <div>
   <div
-    class="container"
+    class="container dashboard-scroll-container"
   >
     <div>
       EditBar
@@ -77,7 +77,7 @@ exports[`NewDashboard renders dashboard 1`] = `
 exports[`NewDashboard renders print preview 1`] = `
 <div>
   <div
-    class="container"
+    class="container dashboard-scroll-container"
   >
     <div>
       EditBar

diff --git a/src/components/Dashboard/__tests__/__snapshots__/ViewDashboard.spec.js.snap b/src/components/Dashboard/__tests__/__snapshots__/ViewDashboard.spec.js.snap
@@ -10,6 +10,7 @@ exports[`ViewDashboard renders correctly default 1`] = `
 >
   <div
     className="container dashboard-scroll-container"
+    data-test="outer-scroll-container"
   >
     <MockDashboardsBar
       expanded={false}

diff --git a/src/components/Dashboard/styles/EditDashboard.module.css b/src/components/Dashboard/styles/EditDashboard.module.css
@@ -17,3 +17,9 @@
         display: block;
     }
 }
+
+@media only screen and (max-height: 480px) {
+    .container {
+        overflow-y: auto;
+    }
+}
diff --git a/src/components/Dashboard/styles/NewDashboard.module.css b/src/components/Dashboard/styles/NewDashboard.module.css
@@ -17,3 +17,9 @@
         display: block;
     }
 }
+
+@media only screen and (max-height: 480px) {
+    .container {
+        overflow-y: auto;
+    }
+}
diff --git a/src/widgets/NoContentMessage.js b/src/widgets/NoContentMessage.js
@@ -1,19 +1,10 @@
 import React from 'react'
-import { colors } from '@dhis2/ui'
 import PropTypes from 'prop-types'
 
+import classes from './styles/NoContentMessage.module.css'
+
 const NoContentMessage = ({ text }) => (
-    <div
-        style={{
-            padding: '50px 10px',
-            textAlign: 'center',
-            fontSize: '15px',
-            fontWeight: 500,
-            color: colors.grey600,
-        }}
-    >
-        {text}
-    </div>
+    <div className={classes.container}>{text}</div>
 )
 
 NoContentMessage.propTypes = {

diff --git a/src/widgets/styles/NoContentMessage.module.css b/src/widgets/styles/NoContentMessage.module.css
@@ -0,0 +1,7 @@
+.container {
+    padding: var(--spacers-dp48) var(--spacers-dp8) 0 var(--spacers-dp8);
+    text-align: center;
+    font-size: 15px;
+    font-weight: 500;
+    color: var(--colors-grey600);
+}