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: Chart sizing #1944

Merged
merged 4 commits into from
Jan 7, 2025
Merged

fix: Chart sizing #1944

merged 4 commits into from
Jan 7, 2025

Conversation

bprusinowski
Copy link
Collaborator

@bprusinowski bprusinowski commented Jan 6, 2025

Fixes #1939

This PR fixes some chart container sizing issues by:

  • re-mounting the container when chart type changes, which fixes errors most noticeable in bar charts, which were scrollable when not needed (due to taking an old height value from the old, mounted container),
  • uses flex display for chart container, which makes the table chart correctly take up 100% of the remaining parent container, not 100% of the parent container, which was resulting in cut scrollbars and double scroll.

How to test

  1. Go to this link.
  2. Switch to a table chart.
  3. ✅ See that the Total number of lines: 389 note is visible in the right bottom corner of the chart.
  4. ✅ Scroll the table and see that you can scroll it all the way down and see the scrollbar at all times.
  5. Resize the browser window so that you can scroll the table horizontally.
  6. ✅ See that the horizontal scrollbar is always visible.

How to reproduce

  1. Go to this link.
  2. Switch to a table chart.
  3. ❌ See that the Total number of lines: 389 note is not visible in the right bottom corner of the chart.
  4. ❌ Scroll the table and see that you can scroll it all the way down, but the scrollbar is cut near the bottom.
  5. Resize the browser window so that you can scroll the table horizontally.
  6. ❌ See that the horizontal scrollbar is not visible.

...to prevent issues with useSize that gets a wrong size from a previous chart when changing chart type that results in scrollable containers.
Copy link

vercel bot commented Jan 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
visualization-tool ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 6, 2025 3:17pm

Copy link
Contributor

@noahonyejese noahonyejese left a comment

Choose a reason for hiding this comment

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

Nice 👍

@bprusinowski bprusinowski merged commit e330c65 into main Jan 7, 2025
6 of 10 checks passed
@bprusinowski bprusinowski deleted the fix/chart-sizing branch January 7, 2025 09:01
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.

Scrollbars in tables cut off
2 participants