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

fix: set dataLabels.padding to 0 to avoid labels being cut off in pie charts #1680

Closed

Conversation

HendrikThePendric
Copy link
Contributor

@HendrikThePendric HendrikThePendric commented Jun 12, 2024

Implements DHIS2-16505

Relates to a yet-to-be-created PR in DV


Key features

  1. Prevent labels being cut off in pie charts

Description

Setting the padding to zero fixes the reported issue.

As far as I am aware, there is no way of knowing exactly when the label is going to get cut off. As such this is an all-or-nothing type of solution: we set padding to 0 for all pie-charts, or we keep things the way they are.

I have checked a few other pie-charts and vieports sizes and do not see any visual degaradations/changes due to this change, so I suggest we make this change.


Reproduction path

  1. Open Pie Chart "Immunization: Fully Immunized by Facility Type" in the data-visualizer-app
  2. Reduce the viewport height to ~375px
  3. Observe that the bottom label is partially cut off

Test the fix

  1. Download the attached custom DV bundle (version 999.9.9-2024-06-12T15-12-27) with this fix included
  2. Install it to a DHIS2 Core instance
  3. Follow the reproduction path above and observe that the bottom label is now fully visible

Screenshots

before

Before

after

After

@martinkrulltott
Copy link
Contributor

Code approved. Needs thorough testing in both DV and Dashboards though before merging

@HendrikThePendric
Copy link
Contributor Author

Closing this PR because the fix didn't work reliably. It just happened to fix the issue for a particular chart at a particular height.

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