-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Fix weather forecast lovelace card documentation #39288
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
Conversation
Since the forecast_type is required it does not make sense to give a default. Leaving it out didn’t work for me. The show_current option is shown in an example and it works for me to configure it, thus list it.
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughA new optional boolean property, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WeatherForecastCard
User->>WeatherForecastCard: Provide YAML config (with/without show_current)
WeatherForecastCard->>WeatherForecastCard: Read show_current (default true)
alt show_current is true
WeatherForecastCard->>User: Display current weather
else show_current is false
WeatherForecastCard->>User: Hide current weather
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
source/_dashboards/weather-forecast.markdown
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - home-assistant-docs
- GitHub Check: Header rules - home-assistant-docs
- GitHub Check: Pages changed - home-assistant-docs
show_current: | ||
required: false | ||
description: Show current wheather. | ||
type: boolean | ||
default: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix typo in show_current
description and surface it in UI docs
The description
reads “Show current wheather.” – please correct the spelling to “Show current weather.”
Also, now that show_current
is documented in YAML, it should be exposed in the Card settings section (around lines 29–31) as a “Show current” toggle with a matching description for the UI editor.
🤖 Prompt for AI Agents
In source/_dashboards/weather-forecast.markdown around lines 64 to 68, fix the
typo in the description of `show_current` by changing "wheather" to "weather".
Then, in the Card settings section near lines 29 to 31, add a toggle control
labeled "Show current" with the corrected description so that this setting is
visible and editable in the UI editor.
📝 WalkthroughWalkthroughA new optional boolean configuration option, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WeatherForecastCard
User->>WeatherForecastCard: Provide YAML configuration (with/without show_current)
WeatherForecastCard->>WeatherForecastCard: Parse show_current (default true if not set)
WeatherForecastCard->>WeatherForecastCard: Determine forecast_type (no default logic)
WeatherForecastCard->>User: Display weather (current shown if show_current is true)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
source/_dashboards/weather-forecast.markdown (1)
64-68
: Fix typo and clarify description forshow_current
The description has a typo ("wheather") and could be enhanced to specify that it controls the display of current weather conditions above the forecast.
- description: Show current wheather. + description: Show the current weather conditions above the forecast.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
source/_dashboards/weather-forecast.markdown
(1 hunks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @maralorn 👍
../Frenck
Since the forecast_type is required it does not make sense to give a default. Leaving it out didn’t work for me.
The show_current option is shown in an example and it works for me to configure it, thus list it.
Proposed change
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit