Skip to content

Commit

Permalink
feat: view mode dashboards-bar design changes [DHIS2-18441] (#3155)
Browse files Browse the repository at this point in the history
Implements JIRA issue DHIS2-18441 according to the design specs

---

1. New design
2. New navigation menu
3. New keyboard navigation

---

Implements the new design according to the design doc. Some other changes are also included in this
PR and these are listed below.

- Use the `useAlert` hook instead of rendering a `AlertStack`. This ensures the show/hide animation
  runs and that there won't be problems when we show this alert together with another alert
- The logic to load a dashboard has been simplified, I suspect this component still contained some
  logic that predates the inclusion of react-router. In any case, there was some logic present that
  accommodated "loading a new dashboard". But this scenario would never occur, because when the
  route changes the component is remounted.
- Instead of having a `getContent` function, we now have the `ViewDashboardContent` component.

A `useEffect` hook was added to ensure that the selected dashboard is cleared from the redux store
when the user  chooses "Close dashboard" in the "action menu" dropdown.

The design was updated and some modifications were done to the behaviour as well:
- When online on a large screen it is possible to edit and/or remove a filter
- When online on a small screen (width =< 480px) it is not possible to edit a filter, but it can be
  removed. When attempting to edit a filter a tooltip shows to inform the user this is not possible.
- When offline filters cannot be edited or deleted. The filter-badge is still visible but the
  remove-icon is not rendered. When attempting to edit a filter a tooltip shows to inform the user
  this is not possible.

Version 10 includes some improvements in terms of accessibility and keyboard navigation. To comply
with the new guidelines in that regard a few small tweaks to components were needed. This involved
adding things like `ariaLabel` props, and addressing some invalid DOM nesting (nested `<a>` tags).

- React testing-library was configured to work with the `data-test` attribute we use at DHIS2 (i.e.
  the `dataTest` prop for components from `@dhis2/ui`). The default data-attribute used by React
  testing-library is `data-test-id`, and some tests needed to be adjusted to no longer use that.
- Some ESLint rules were added specifically for `spec.js` to remove the need to add ESLint ignore
  comments when making mock-components and other types of mocks.

---

-   Keyboard navigation inconsistencies: this PR includes a major version bump in `@dhis2/ui` which
    comes with some accessibility improvements including arrow-key keyboard navigation for menus.
    The navigation- and more-actions-menu in the new dashboards-bar are using these. But the
    filter-menu is using a custom menu-list and this can be used by keyboard by using the tab-key.
    This inconsistency will be tackled in a separate PR as described in JIRA issue DHIS2-18537.

---

Current JIRA Issue DHIS2-18441: https://dhis2.atlassian.net/browse/DHIS2-18441
Follow-up JIRA issue DHIS2-18537: https://dhis2.atlassian.net/browse/DHIS2-18537
Design specs: https://docs.google.com/document/d/1c8Ll1aLbFYwU8HYsyDlH1wk_QKTrUXwNanCiqayczOM
  • Loading branch information
HendrikThePendric committed Dec 18, 2024
1 parent b890115 commit 1776ac5
Show file tree
Hide file tree
Showing 145 changed files with 2,297 additions and 3,722 deletions.
11 changes: 11 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,15 @@ const { config } = require('@dhis2/cli-style')

module.exports = {
extends: [config.eslintReact, 'plugin:cypress/recommended'],
overrides: [
{
files: ['src/**/*.spec.js'],
rules: {
'react/prop-types': 'off',
'react/display-name': 'off',
'react/no-unknown-property': 'off',
'no-unused-vars': ['error', { ignoreRestSiblings: true }],
},
},
],
}
6 changes: 6 additions & 0 deletions config/testSetup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { configure } from '@testing-library/dom'
import '@testing-library/jest-dom'

configure({
testIdAttribute: 'data-test',
})
2 changes: 1 addition & 1 deletion cypress/e2e/common/add_a_FILTERTYPE_filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const OU_ID = 'ImspTQPwCqd' //Sierra Leone
const FACILITY_TYPE = 'Clinic'

When('I add a {string} filter', (dimensionType) => {
cy.contains('Add filter').click()
cy.containsExact('Filter').click()

// select an item in the modal
switch (dimensionType) {
Expand Down
5 changes: 4 additions & 1 deletion cypress/e2e/common/click_on_the_FILTERTYPE_filter_badge.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,8 @@ import { When } from '@badeball/cypress-cucumber-preprocessor'
import { filterBadgeSel } from '../../elements/dashboardFilter.js'

When('I click on the {string} filter badge', (filterName) => {
cy.get(filterBadgeSel).find('span:visible').contains(filterName).click()
cy.get(filterBadgeSel)
.find('button')
.contains(filterName)
.click({ force: true })
})
3 changes: 1 addition & 2 deletions cypress/e2e/common/open_print_layout.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { When } from '@badeball/cypress-cucumber-preprocessor'
import { clickViewActionButton } from '../../elements/viewDashboard.js'

When('I click to preview the print layout', () => {
clickViewActionButton('More')
cy.get('[data-test="more-actions-button"]').click()
cy.get('[data-test="print-menu-item"]').click()
cy.get('[data-test="print-layout-menu-item"]').click()
})
9 changes: 3 additions & 6 deletions cypress/e2e/common/open_the_SL_dashboard.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import { Given } from '@badeball/cypress-cucumber-preprocessor'
import { dashboards } from '../../assets/backends/index.js'
// import { gridItemSel, chartSel } from '../../elements/dashboardItem.js'
import {
dashboardTitleSel,
dashboardChipSel,
} from '../../elements/viewDashboard.js'
import { EXTENDED_TIMEOUT } from '../../support/utils.js'
import { getNavigationMenuItem } from '../../elements/navigationMenu.js'
import { dashboardTitleSel } from '../../elements/viewDashboard.js'

Given('I open the {string} dashboard', (title) => {
cy.get(dashboardChipSel, EXTENDED_TIMEOUT).contains(title).click()
getNavigationMenuItem(title).click()

cy.location().should((loc) => {
expect(loc.hash).to.equal(dashboards[title].route)
Expand Down
10 changes: 3 additions & 7 deletions cypress/e2e/dashboard_filter/create_dashboard.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { Given, When, Then } from '@badeball/cypress-cucumber-preprocessor'
import { gridItemSel } from '../../elements/dashboardItem.js'
import {
dashboardChipSel,
dashboardTitleSel,
} from '../../elements/viewDashboard.js'
import { getNavigationMenuItem } from '../../elements/navigationMenu.js'
import { dashboardTitleSel } from '../../elements/viewDashboard.js'
import {
EXTENDED_TIMEOUT,
createDashboardTitle,
Expand Down Expand Up @@ -79,9 +77,7 @@ When('I add items and save', () => {
})

Given('I open an existing dashboard', () => {
cy.get(dashboardChipSel, EXTENDED_TIMEOUT)
.contains(TEST_DASHBOARD_TITLE)
.click()
getNavigationMenuItem(TEST_DASHBOARD_TITLE).click()
})

// Some map visualization load very slowly:
Expand Down
3 changes: 2 additions & 1 deletion cypress/e2e/dashboard_filter/dashboard_filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Then, When } from '@badeball/cypress-cucumber-preprocessor'
import {
filterBadgeSel,
dimensionsModalSel,
filterBadgeDeleteBtnSel,
} from '../../elements/dashboardFilter.js'
// import {
// gridItemSel,
Expand Down Expand Up @@ -128,7 +129,7 @@ Then('the filter modal is opened', () => {
})

When('I remove the {string} filter', () => {
cy.get(filterBadgeSel).find('button').contains('Remove').click()
cy.get(filterBadgeDeleteBtnSel).click()
})

Then('the filter is removed from the dashboard', () => {
Expand Down
11 changes: 5 additions & 6 deletions cypress/e2e/edit_dashboard/edit_dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import {
titleInputSel,
clickEditActionButton,
} from '../../elements/editDashboard.js'
import { getNavigationMenuItem } from '../../elements/navigationMenu.js'
import {
dashboardChipSel,
dashboardTitleSel,
dashboardsNavMenuButtonSel,
} from '../../elements/viewDashboard.js'
import { EXTENDED_TIMEOUT, createDashboardTitle } from '../../support/utils.js'

Expand Down Expand Up @@ -79,9 +80,8 @@ Then('different valid dashboard displays in view mode', () => {
})

Given('I open existing dashboard', () => {
cy.get(dashboardChipSel, EXTENDED_TIMEOUT)
.contains(TEST_DASHBOARD_TITLE)
.click()
cy.get(dashboardsNavMenuButtonSel, EXTENDED_TIMEOUT).click()
cy.get('[role="menu"]').find('li').contains(TEST_DASHBOARD_TITLE).click()

cy.location().should((loc) => {
const currentRoute = getRouteFromHash(loc.hash)
Expand Down Expand Up @@ -124,8 +124,7 @@ Scenario: I delete a dashboard
*/

Then('the dashboard is deleted and first starred dashboard displayed', () => {
cy.get(dashboardChipSel).contains(TEST_DASHBOARD_TITLE).should('not.exist')

getNavigationMenuItem(TEST_DASHBOARD_TITLE).should('not.exist')
cy.get(dashboardTitleSel).should('exist').should('not.be.empty')
})

Expand Down
29 changes: 15 additions & 14 deletions cypress/e2e/edit_dashboard/star_dashboard.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
import { When, Then } from '@badeball/cypress-cucumber-preprocessor'
import {
starSel,
getNavigationMenuItem,
closeNavigationMenu,
} from '../../elements/navigationMenu.js'
import {
dashboardStarredSel,
dashboardUnstarredSel,
dashboardChipSel,
chipStarSel,
navMenuItemStarIconSel,
} from '../../elements/viewDashboard.js'
import { TEST_DASHBOARD_TITLE } from './edit_dashboard.js'

// Scenario: I star the dashboard
When('I click to star the dashboard', () => {
cy.intercept('POST', '**/favorite').as('starDashboard')

cy.get(starSel).click()
cy.get(dashboardUnstarredSel).click()
cy.wait('@starDashboard').its('response.statusCode').should('eq', 200)
})

When('I click to unstar the dashboard', () => {
cy.intercept('DELETE', '**/favorite').as('unstarDashboard')

cy.get(starSel).click()
cy.get(dashboardStarredSel).click()
cy.wait('@unstarDashboard').its('response.statusCode').should('eq', 200)
})

Expand All @@ -28,22 +30,21 @@ Then('the dashboard is starred', () => {
cy.get(dashboardStarredSel).should('be.visible')
cy.get(dashboardUnstarredSel).should('not.exist')

cy.get(dashboardChipSel)
.contains(TEST_DASHBOARD_TITLE)
.parent()
.siblings(chipStarSel)
.first()
getNavigationMenuItem(TEST_DASHBOARD_TITLE)
.find(navMenuItemStarIconSel)
.should('be.visible')

closeNavigationMenu()
})

Then('the dashboard is not starred', () => {
// check for the unfilled star next to the title
cy.get(dashboardUnstarredSel).should('be.visible')
cy.get(dashboardStarredSel).should('not.exist')

cy.get(dashboardChipSel)
.contains(TEST_DASHBOARD_TITLE)
.parent()
.siblings()
getNavigationMenuItem(TEST_DASHBOARD_TITLE)
.find(navMenuItemStarIconSel)
.should('not.exist')

closeNavigationMenu()
})
4 changes: 2 additions & 2 deletions cypress/e2e/filter_restrict/filter_restrict.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ When('I save the dashboard', () => {
})

When('I click Add Filter', () => {
clickViewActionButton('Add filter')
clickViewActionButton('Filter')
})

Then('I see Facility Ownership and no other dimensions', () => {
Expand All @@ -168,7 +168,7 @@ Scenario: I restrict filters to no dimensions and do not see Add Filter in dashb
*/

Then('Add Filter button is not visible', () => {
cy.contains('Add filter').should('not.exist')
cy.containsExact('Filter').should('not.exist')
})

When('I delete the dashboard', () => {
Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/offline/offline.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ Then(
// edit, sharing, starring, filtering, all options under more
getViewActionButton('Edit').should('be.disabled')
getViewActionButton('Share').should('be.disabled')
getViewActionButton('Add filter').should('be.disabled')
getViewActionButton('Filter').should('be.disabled')
getViewActionButton('More').should('be.enabled')

checkCorrectMoreOptionsEnabledState(false, cacheState)
Expand Down
10 changes: 2 additions & 8 deletions cypress/e2e/responsive_dashboard/responsive_dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ Then('the small screen view is shown', () => {
//titlebar - only the More button and the title
cy.get('button').contains('Edit').should('not.be.visible')
cy.get('button').contains('Share').should('not.be.visible')
cy.get('button').contains('Add filter').should('not.be.visible')

cy.get('button.small').contains('More').should('be.visible')
cy.get('button').not('.small').contains('More').should('not.be.visible')
cy.get('button').contains('Filter').should('not.be.visible')
})

When('I restore the wide screen', () => {
Expand All @@ -44,10 +41,7 @@ Then('the wide screen view is shown', () => {

cy.get('button').contains('Edit').should('be.visible')
cy.get('button').contains('Share').should('be.visible')
cy.get('button').contains('Add filter').should('be.visible')

cy.get('button').not('.small').contains('More').should('be.visible')
cy.get('button.small').contains('More').should('not.be.visible')
cy.get('button').contains('Filter').should('be.visible')
})

Then('the small screen edit view is shown', () => {
Expand Down
19 changes: 1 addition & 18 deletions cypress/e2e/view_dashboard.feature
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,14 @@ Feature: Viewing dashboards
Given I open the "Antenatal Care" dashboard
When I search for dashboards containing "Immun"
Then Immunization and Immunization data dashboards are choices
When I press enter in the search dashboard field
When I press tab in the search dashboard field and then enter
Then the "Immunization" dashboard displays in view mode

@nonmutating
Scenario: I search for a dashboard with nonmatching search text
Given I open the "Antenatal Care" dashboard
When I search for dashboards containing "Noexist"
Then no dashboards are choices
When I press enter in the search dashboard field
Then dashboards list restored and dashboard is still "Antenatal Care"

@nonmutating
Scenario: I view the print layout preview and then print one-item-per-page preview
Expand All @@ -39,21 +37,6 @@ Feature: Viewing dashboards
Given I open the "Delivery" dashboard with shapes removed
Then the "Delivery" dashboard displays in view mode

@nonmutating
Scenario: I expand the control bar
Given I open the "Delivery" dashboard
Then the control bar should be at collapsed height
When I toggle show more dashboards
Then the control bar should be expanded to full height

@nonmutating
Scenario: I expand the control bar when dashboard not found
Given I type an invalid dashboard id in the browser url
Then a message displays informing that the dashboard is not found
And the control bar should be at collapsed height
When I toggle show more dashboards
Then the control bar should be expanded to full height

# @nonmutating
# FIXME
# Scenario: Maps with tracked entities show layer names in legend
Expand Down
5 changes: 2 additions & 3 deletions cypress/e2e/view_dashboard/dashboard_items_without_shape.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Given } from '@badeball/cypress-cucumber-preprocessor'
import { dashboards } from '../../assets/backends/index.js'
import { dashboardChipSel } from '../../elements/viewDashboard.js'
import { EXTENDED_TIMEOUT } from '../../support/utils.js'
import { getNavigationMenuItem } from '../../elements/navigationMenu.js'

Given('I open the {string} dashboard with shapes removed', (title) => {
const regex = new RegExp(`dashboards/${dashboards[title].id}`, 'g')
Expand All @@ -17,5 +16,5 @@ Given('I open the {string} dashboard with shapes removed', (title) => {
res.send({ body: res.body })
})
})
cy.get(dashboardChipSel, EXTENDED_TIMEOUT).contains(title).click()
getNavigationMenuItem(title).click()
})
6 changes: 1 addition & 5 deletions cypress/e2e/view_dashboard/open_dashboard_app.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { When, Then } from '@badeball/cypress-cucumber-preprocessor'
import {
dashboardTitleSel,
dashboardChipSel,
} from '../../elements/viewDashboard.js'
import { dashboardTitleSel } from '../../elements/viewDashboard.js'
import { EXTENDED_TIMEOUT } from '../../support/utils.js'

When('I open the dashboard app with the root url', () => {
Expand All @@ -13,7 +10,6 @@ When('I open the dashboard app with the root url', () => {
})

cy.get(dashboardTitleSel).should('be.visible')
cy.get(dashboardChipSel, EXTENDED_TIMEOUT).should('be.visible')
})

Then('the {string} dashboard displays', (title) => {
Expand Down
3 changes: 1 addition & 2 deletions cypress/e2e/view_dashboard/print.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { When, Then } from '@badeball/cypress-cucumber-preprocessor'
import { dashboards } from '../../assets/backends/sierraLeone_236.js'
import { clickViewActionButton } from '../../elements/viewDashboard.js'

When('I click to preview the print one-item-per-page', () => {
clickViewActionButton('More')
cy.get('[data-test="more-actions-button"]').click()
cy.get('[data-test="print-menu-item"]').click()
cy.get('[data-test="print-oipp-menu-item"]').click()
})
Expand Down
54 changes: 0 additions & 54 deletions cypress/e2e/view_dashboard/resize_dashboards_bar.js

This file was deleted.

Loading

0 comments on commit 1776ac5

Please sign in to comment.