-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Update voice language picker #39775
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
Update voice language picker #39775
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Pull Request Overview
This PR refactors the voice language picker to source support data from a centralized JSON, updates the local/cloud comparison graphics and styling, and adds a Rake task to fetch language support scores automatically.
- CSS enhancements for responsive layout and new graphic-card link styling
- JavaScript refactor to drive language support UI from
window.language_scores
- Markup updates to the language selector options and local/cloud feature comparison
- New Rake task to download
language_scores.json
data
Reviewed Changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
source/voice-pe/styles.css | Expanded and refactored CSS for local/cloud cards and lines |
source/voice-pe/script.js | Removed hard-coded support table, bound select placeholder, and used window.language_scores |
source/voice-pe/index.html | Overhauled HTML for language picker and local/cloud graphic sections; injected language_scores |
Rakefile | Added task to fetch remote language_scores.json |
Comments suppressed due to low confidence (2)
source/voice-pe/styles.css:2155
- Missing semicolon at the end of this declaration, which will produce a CSS syntax error. Add
;
after the value.
height: calc(var(--grid-m) * 16)
source/voice-pe/index.html:681
- Duplicate
<option>
entry forpt-BR
. Remove the redundant option to prevent confusing the select choices.
<option value="pt-BR">Portuguese (Brazil)</option>
📝 WalkthroughWalkthroughThe updates introduce a new Rake task to fetch language score data, update the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Rake Task
participant Server (ohf-voice.github.io)
participant Browser
User->>Rake Task: Run language_scores_data task
Rake Task->>Server (ohf-voice.github.io): GET /intents/language_scores.json
Server (ohf-voice.github.io)-->>Rake Task: Return language_scores.json
Rake Task->>File System: Save as source/_data/language_scores.json
User->>Browser: Load Voice Preview Edition page
Browser->>Browser: Inline script exposes window.language_scores
Browser->>Browser: updateLanguageSupports() uses window.language_scores to update UI
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (5)
✨ Finishing Touches
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: 4
🔭 Outside diff range comments (1)
source/voice-pe/script.js (1)
462-508
: Add error handling for missing language scores data.The refactored function logic is sound, but it should handle cases where
window.language_scores
is undefined or empty.Add defensive programming:
function updateLanguageSupports(locale = null) { + if (!window.language_scores) { + console.warn('Language scores data not available'); + return; + } let data = window.language_scores;
🧹 Nitpick comments (2)
source/voice-pe/styles.css (2)
2188-2203
: Hard-coded negative margins risk horizontal scrollThe new
.devices
block usesmargin-left: -120px;
which can create unwanted horizontal overflow on narrow viewports or when--grid-m
changes.
Consider using flex/grid alignment rather than magic numbers.
2390-2399
:line-dashed
animation: colour hard-coded black
background
isblack
, whereas the rest of the design derives colours from custom props.
Exposing it asvar(--color)
or a dedicated variable would keep themes consistent.
📜 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 ignored due to path filters (3)
source/images/voice-pe/local-cloud/ha-instance.svg
is excluded by!**/*.svg
source/images/voice-pe/local-cloud/house.svg
is excluded by!**/*.svg
source/voice-pe/rounded-path.svg
is excluded by!**/*.svg
📒 Files selected for processing (5)
.gitignore
(1 hunks)Rakefile
(1 hunks)source/voice-pe/index.html
(10 hunks)source/voice-pe/script.js
(3 hunks)source/voice-pe/styles.css
(23 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
🔇 Additional comments (7)
.gitignore (1)
11-11
: LGTM! Correctly excluding generated data file.This addition follows the established pattern of ignoring generated JSON data files and aligns with the new Rake task that downloads language scores data.
source/voice-pe/script.js (2)
31-31
: LGTM! Placeholder update is consistent.The placeholder change aligns with the HTML updates and improves user experience.
449-460
: Well-structured language descriptions.The languageDescriptions object provides clear, informative descriptions for different support states and makes the code more maintainable.
source/voice-pe/index.html (3)
30-31
: LGTM! Documentation links updated correctly.The documentation URL updates point to the correct support categories.
Also applies to: 44-45
467-468
: Good content improvement.The revised text is more concise and clear.
634-698
: Improved language selection with descriptive names.The updated language options with country/region annotations provide better user experience and clarity.
source/voice-pe/styles.css (1)
479-485
: z-index raise to 6 – verify stacking contextBumping
.vpe-nav
toz-index: 6
may place it above the modal (.video-modal
usesz-index: 10
) but could still eclipse other overlays added later.
Please confirm there are no pop-ups/tooltips in the range7-9
, or pick a constant from a shared scale.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Proposed change
Updates the voice language picker to use the new data sources
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit
New Features
Enhancements
Style
Chores