fix: improved wind indicator font and tip visibility#31
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the wind indicator component to improve arrow visibility by simplifying the arrow geometry and centralizing wind speed unit handling. The component now uses a simpler coordinate-based polygon for the arrow instead of polar coordinates, and delegates unit conversion to a centralized getNormalizedWindSpeed utility function.
- Refactored wind indicator component API to accept
hass,weatherEntity, andforecastobjects instead of individual wind properties - Simplified arrow rendering using Cartesian coordinates instead of polar math for better visibility
- Added comprehensive test suite covering rendering, rotation, type behavior, colors, and sizing
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/wind-indicator.test.ts | New comprehensive test file with 20+ tests covering rendering, rotation, bearing vs direction types, color thresholds, and custom sizing |
| src/components/wfc-wind-indicator.ts | Refactored component API, simplified arrow geometry using Cartesian coordinates, moved unit conversion to getNormalizedWindSpeed, improved text positioning and accessibility |
| src/components/wfc-forecast-simple.ts | Updated to pass ExtendedHomeAssistant type and weatherEntity prop to child components |
| src/components/wfc-forecast-info.ts | Refactored to pass weatherEntity prop to wind indicator instead of computing wind unit locally |
| src/components/wfc-forecast-chart.ts | Updated to pass weatherEntity prop to forecast info component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
troinine
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/components/wfc-wind-indicator.ts
Outdated
| </g> | ||
|
|
||
| <text x="${cx}" y="${cy + 5}" text-anchor="middle">${speed}</text> | ||
| <text x="${cx}" y="${cy}" dy="2px">${speed}</text> |
There was a problem hiding this comment.
The dy="2px" offset is hardcoded in pixels, which may not scale properly with different font sizes or SVG dimensions. Consider calculating this offset based on the font size or using a unitless value that scales with the viewBox dimensions.
| <text x="${cx}" y="${cy}" dy="2px">${speed}</text> | |
| <text x="${cx}" y="${cy}" dy="0.1em">${speed}</text> |
1 participant
No description provided.