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

Bar Charts with larger scales have wider margins #331

Merged
merged 7 commits into from
Aug 1, 2023
Merged

Conversation

KevinWu098
Copy link
Member

@KevinWu098 KevinWu098 commented Jul 20, 2023

Description

  • this.classData() is set to a new const data so that largeGraphScale can access the array without recalling this.classData()
  • Added new const greatestCount to find the largest grade value within data
  • Added new const marginX which calculates margins depending on the magnitude of greatestCount, with every magnitude above 100 increasing the base margin (30) by 5. For example, 5173 would set marginX to 35, while 123 would set it to 30.

More detailed documentation is in code comments!

Screenshots

Before:
Before Chart

After:
chrome-capture-2023-6-28

Steps to verify/test this change:

  • Verify changes work as expected on staging instance

Final Checks:

  • Verify successful deployment
  • Delete branch

Closes #296

@KevinWu098
Copy link
Member Author

After reviewing the code a bit, I realized that only checking for four digit number would kick the can down the road for five digits etc.

TLDR Wrote some new code to find the largest grade count, then increase the margin size based on the magnitude of that largest count.

@KevinWu098 KevinWu098 requested a review from js0mmer July 28, 2023 10:56
@KevinWu098 KevinWu098 changed the title Bar Charts with 4 digit scales have wider margins Bar Charts with larger scales have wider margins Jul 28, 2023
@github-actions
Copy link

Deployed staging instance to https://staging-331.peterportal.org

Copy link
Member

@js0mmer js0mmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@js0mmer js0mmer merged commit 5909c8d into master Aug 1, 2023
2 checks passed
@js0mmer js0mmer deleted the scaleFourDigit branch August 1, 2023 01:44
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.

Scale on grade distribution chart cut off for 4 digits
2 participants