Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: view mode dashboards-bar design changes [DHIS2-18441] #3155

Conversation

HendrikThePendric
Copy link
Contributor

@HendrikThePendric HendrikThePendric commented Nov 26, 2024

Implements DHIS2-18441 according to design doc


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.

TODO

  • KFMT
  • Add e2e test for error state, i.e. dashboard items request returning a 500

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, see DHIS2-18537.

Screenshots

Screenshot 2024-12-10 at 16 55 19

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Nov 26, 2024

🚀 Deployed on https://pr-3155.dashboard.netlify.dhis2.org

@dhis2-bot dhis2-bot temporarily deployed to netlify November 26, 2024 15:24 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 26, 2024 15:59 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 26, 2024 16:03 Inactive
@HendrikThePendric HendrikThePendric marked this pull request as draft November 26, 2024 16:27
@dhis2-bot dhis2-bot temporarily deployed to netlify November 26, 2024 16:49 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 08:59 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 11:24 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 14:36 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 14:56 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 15:00 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 15:33 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 15:37 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 15:45 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 16:17 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 16:21 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 16:27 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 21:35 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 28, 2024 21:49 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 2, 2024 16:44 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 2, 2024 16:50 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 3, 2024 11:49 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 3, 2024 14:53 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 3, 2024 16:42 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 16, 2024 16:03 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 16, 2024 16:16 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 16, 2024 17:09 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 17, 2024 08:27 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 18, 2024 09:31 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 18, 2024 10:50 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 18, 2024 12:33 Inactive
@HendrikThePendric HendrikThePendric force-pushed the feat/header-bar-design-ui-update branch from 2b8eb74 to 21e9f95 Compare December 18, 2024 12:35
@dhis2-bot dhis2-bot temporarily deployed to netlify December 18, 2024 12:38 Inactive
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
12 New issues
1 New Critical Issues (required ≤ 0)
9 New Code Smells (required ≤ 0)
3 New Bugs (required ≤ 0)
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@dhis2-bot dhis2-bot temporarily deployed to netlify December 18, 2024 12:44 Inactive
@HendrikThePendric HendrikThePendric changed the base branch from master to feat/release-DHIS2-18441-and-DHIS2-13038 December 18, 2024 13:05
@HendrikThePendric HendrikThePendric merged commit de1f429 into feat/release-DHIS2-18441-and-DHIS2-13038 Dec 18, 2024
30 of 31 checks passed
@HendrikThePendric HendrikThePendric deleted the feat/header-bar-design-ui-update branch December 18, 2024 13:36
HendrikThePendric added a commit that referenced this pull request Dec 18, 2024
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
HendrikThePendric added a commit that referenced this pull request Dec 18, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants