Skip to content

Commit

Permalink
feat: add space efficient dashboard bar design and dashboard selectio…
Browse files Browse the repository at this point in the history
…n, and keyboard navigation

Implements JIRA issue DHIS2-18441 according to the design specs

---

### Key features

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

---

### Description

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

#### Changes to `ViewDashboard`

- 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.

#### Changes to `CacheableViewDashboard`

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.

#### Changes to `FilterBadge`

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.

#### Major version upgrade of `@dhis2/ui` (v10)

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).

#### Some tweaks to the unit-testing configuration

- 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.

---

### Known issues

-   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.

---

### References

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 45e6f4c
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 45e6f4c

Please sign in to comment.