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: improve support for right-to-left languages in Dashboard app #2985

Merged
merged 21 commits into from
Sep 26, 2024

Conversation

kabaros
Copy link
Collaborator

@kabaros kabaros commented May 31, 2024

Implements DHIS2-16778

Key features

  1. Use logical CSS properties instead of physical properties throughout the project (done automatically with a tool)
  2. Update the cli-style to a version that supports checking logical properties (and automatically converting physical to logical properties)

Description

This PR implements better support for right-to-left languages in the app, by leveraging the use of logical CSS properties that are independent of language direction, rather than physical properties (i.e. *-start instead of -left and *-end instead *-right). Logical properties are widely supported now and they should become the default way for layout going-forward to ensure that we support RTL languages without extra development burden (which will be enforced by d2 style).

Although the change set is big. It's mostly done automatically using d2 style apply css which under the hood uses stylelint to enforce and apply the rules.

The better support for RTL builds on top of previous work done in the platform, namely:

Testing

  • The main test here should be that the app in English (or any LTR language) should look exactly as before, after these changes.
  • Test on an environment set to Arabic (or any other RTL language) to confirm it looks as expected for an RTL user.

Screenshots

Before After
dashboard_before dashboard_after

Bumps [express](https://github.com/expressjs/express) from 4.18.1 to 4.19.2.
- [Release notes](https://github.com/expressjs/express/releases)
- [Changelog](https://github.com/expressjs/express/blob/master/History.md)
- [Commits](expressjs/express@4.18.1...4.19.2)

---
updated-dependencies:
- dependency-name: express
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
@kabaros kabaros marked this pull request as ready for review June 3, 2024 12:34
@kabaros kabaros requested a review from a team June 3, 2024 14:07
@jenniferarnesen jenniferarnesen changed the base branch from dev to master June 21, 2024 07:53
@dhis2-bot
Copy link
Contributor

dhis2-bot commented Aug 26, 2024

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

@dhis2-bot dhis2-bot temporarily deployed to netlify August 26, 2024 13:26 Inactive
@jenniferarnesen
Copy link
Collaborator

@kabaros There is a lint error in ItemGrid.css. Could you look into it?

@dhis2-bot dhis2-bot temporarily deployed to netlify August 26, 2024 13:47 Inactive
@jenniferarnesen
Copy link
Collaborator

@kabaros Maybe I'm doing something wrong here, but I can't get the app to run locally:
image

@jenniferarnesen
Copy link
Collaborator

@kabaros There is a lint error in ItemGrid.css. Could you look into it?

I went ahead and changed it according to the recommendation. Please check that my change is correct.

@dhis2-bot dhis2-bot temporarily deployed to netlify August 27, 2024 15:49 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify September 26, 2024 07:28 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify September 26, 2024 07:48 Inactive
@jenniferarnesen jenniferarnesen added the e2e record Apply this label to a pull request to trigger recording of E2E tests on Cypress Cloud label Sep 26, 2024
Copy link

cypress bot commented Sep 26, 2024

dashboards-app    Run #4997

Run Properties:  status check passed Passed #4997  •  git commit 0c28a71b0d ℹ️: Merge 00c71016761b5c3b0e34336052c4363d278145bb into 85b3da89489ddf3c3ef86af1a439...
Project dashboards-app
Branch Review DHIS2-16777/rtl-support
Run status status check passed Passed #4997
Run duration 01m 45s
Commit git commit 0c28a71b0d ℹ️: Merge 00c71016761b5c3b0e34336052c4363d278145bb into 85b3da89489ddf3c3ef86af1a439...
Committer Mozafar
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 8
View all changes introduced in this branch ↗︎

@jenniferarnesen
Copy link
Collaborator

jenniferarnesen commented Sep 26, 2024

Found a bug. In print one item per page, the visualizations are not resizing to the full height of the container. It looks like they maintain the same height as they had in the normal dashboard view:
image

@jenniferarnesen
Copy link
Collaborator

jenniferarnesen commented Sep 26, 2024

Bug: When adding an item to the dashboard, the visualization height is not filling the height of the containing item:
image

@jenniferarnesen
Copy link
Collaborator

Bug. After saving a new dashboard, most items get the correct height in view mode, but not the App item (e.g. scorecard widget):
image

@jenniferarnesen
Copy link
Collaborator

jenniferarnesen commented Sep 26, 2024

Bug: messages and text items don't have scrollbar (both desktop and small screen views):
image

image

@jenniferarnesen
Copy link
Collaborator

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

Use this url for testing:
https://pr-2985--dhis2-dashboard.netlify.app/

@dhis2-bot dhis2-bot temporarily deployed to netlify September 26, 2024 10:59 Inactive
@jenniferarnesen
Copy link
Collaborator

All the bugs reported here are now fixed.

@jenniferarnesen jenniferarnesen merged commit 9b3b585 into master Sep 26, 2024
43 checks passed
@jenniferarnesen jenniferarnesen deleted the DHIS2-16777/rtl-support branch September 26, 2024 13:28
dhis2-bot added a commit that referenced this pull request Sep 26, 2024
# [100.3.0](v100.2.5...v100.3.0) (2024-09-26)

### Features

* improve support for right-to-left languages in Dashboard app ([#2985](#2985)) ([9b3b585](9b3b585)), closes [ui#1448](https://github.com/ui/issues/1448) [app-platform#825](https://github.com/app-platform/issues/825) [cli-style#464](https://github.com/cli-style/issues/464)
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 100.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e record Apply this label to a pull request to trigger recording of E2E tests on Cypress Cloud released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants