Skip to content

Conversation

@tycup
Copy link
Contributor

@tycup tycup commented Oct 27, 2025

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Oct 27, 2025

⚠️ No Changeset found

Latest commit: 59e9877

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Oct 27, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
saltdesignsystem Ready Ready Preview Comment Nov 6, 2025 4:00pm

@amritadesmet
Copy link
Contributor

Looking good! I just noticed that the X axis title 'Category' isn't centre aligned to the main chart title 'Regional revenue by product' and looks visually off - is this something that can be adjusted or is this built in? @tycup

@tycup
Copy link
Contributor Author

tycup commented Oct 28, 2025

Looking good! I just noticed that the X axis title 'Category' isn't centre aligned to the main chart title 'Regional revenue by product' and looks visually off - is this something that can be adjusted or is this built in? @tycup

Built in - to do with how space for x axis labels is distrbuted - you can see the same in the demo: https://www.highcharts.com/demo/highcharts/waterfall

@amritadesmet
Copy link
Contributor

Looking good! I just noticed that the X axis title 'Category' isn't centre aligned to the main chart title 'Regional revenue by product' and looks visually off - is this something that can be adjusted or is this built in? @tycup

Built in - to do with how space for x axis labels is distrbuted - you can see the same in the demo: https://www.highcharts.com/demo/highcharts/waterfall

Yeah strange, it looks like they've removed the X axis title to make it look visually right? In Figma I've aligned the titles centred to each other and will make a note on the frame of the visual discrepancy in code

@tycup tycup marked this pull request as ready for review October 30, 2025 14:42
joshwooding
joshwooding previously approved these changes Oct 31, 2025
@amritadesmet
Copy link
Contributor

Looking good! I just noticed that the X axis title 'Category' isn't centre aligned to the main chart title 'Regional revenue by product' and looks visually off - is this something that can be adjusted or is this built in? @tycup

Built in - to do with how space for x axis labels is distrbuted - you can see the same in the demo: https://www.highcharts.com/demo/highcharts/waterfall

Yeah strange, it looks like they've removed the X axis title to make it look visually right? In Figma I've aligned the titles centred to each other and will make a note on the frame of the visual discrepancy in code

Could you add a note of this visual discrepancy in the info banner at the top, similar to what you did for Legends? @tycup

@joshwooding joshwooding added the chromatic Run chromatic on the current PR. Will be removed by the CI once submitted. label Nov 6, 2025
@github-actions github-actions bot removed the chromatic Run chromatic on the current PR. Will be removed by the CI once submitted. label Nov 6, 2025
@joshwooding joshwooding merged commit aa99da7 into highcharts Nov 7, 2025
13 of 15 checks passed
@joshwooding joshwooding deleted the highcharts-waterfall branch November 7, 2025 11:45
joshwooding added a commit that referenced this pull request Nov 12, 2025
tycup added a commit that referenced this pull request Nov 25, 2025
tycup added a commit that referenced this pull request Dec 3, 2025
tycup added a commit that referenced this pull request Dec 3, 2025
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