fix: adjust chart temperature boundaries to avoid label overflow#24
Merged
fix: adjust chart temperature boundaries to avoid label overflow#24
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #14 where temperature labels in chart mode would overflow over the x-axis when displaying low temperatures, particularly negative values. The fix enhances the algorithm that computes Y-axis boundaries to provide more bottom padding for low-temperature labels.
Key Changes:
- Improved the
computeScaleLimits()algorithm with better padding calculations (35% bottom vs 20% top) and a minimum 5° buffer for low temperatures - Harmonized and renamed CSS variables to follow a consistent
--wfc-chart-*naming pattern for better customization - Added comprehensive test data with problematic negative temperature values to prevent regression
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/mocks/test-data.ts | New test data file with two datasets (ISSUE_14_DAILY_FORECAST and ISSUE_14_DAILY_FORECAST_2) containing negative temperature values that reproduce the label overflow bug |
| test/mocks/hass.ts | Enhanced mock with proper TypeScript types, public forecast properties for test data injection, and improved type safety for forecast subscriptions |
| test/app/index.html | Added two new test cards (test-card-bug-14 and test-card-bug-14-2) that use the regression test data to verify the fix |
| src/weather-forecast-card.css | Renamed CSS variables to follow consistent --wfc-chart-* naming pattern and added separate label color variables for high temps, low temps, and precipitation |
| src/components/wfc-forecast-chart.ts | Updated chart configuration to use new CSS variable names, improved computeScaleLimits() algorithm with asymmetric padding (35% bottom, 20% top) and higher minimum buffers, and enhanced documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Tero Roininen <troinine@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Tero Roininen <troinine@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change fixes #14 where low temperature line graph caused labels to overflow over x-axis.
Additionally, it harmonizes and improves chart CSS variables for better custom styling of the graph.