Fix service advanced options UI #21925
Conversation
WalkthroughWalkthroughThe changes involve a modification to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HaServiceControl
participant Field
User->>HaServiceControl: Request fields
HaServiceControl->>Field: Check top-level fields for selector
alt Has selector
Field-->>HaServiceControl: Return selector found
else No selector
Field-->>HaServiceControl: Return no selector
end
HaServiceControl->>Field: Check nested fields for selector
alt Has nested selector
Field-->>HaServiceControl: Return nested selector found
else No nested selector
Field-->>HaServiceControl: Return no nested selector
end
HaServiceControl-->>User: Return hasSelector data
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
|
||
| const hasSelector: string[] = []; | ||
| fields.forEach((field) => { | ||
| if ((field as any).fields) { |
There was a problem hiding this comment.
Could not get the typing figured out here so had to avoid it. It seems like we're doing some type hacking in the header to try to make this work, but I think what we probably need is an update to home-assistant-js-websocket to properly type fields with subfields?
There was a problem hiding this comment.
| const hasSelector: string[] = []; | ||
| fields.forEach((field) => { | ||
| if ((field as any).fields) { | ||
| Object.entries((field as any).fields).forEach(([key, subField]) => { | ||
| if ((subField as any).selector) { | ||
| hasSelector.push(key); | ||
| } | ||
| }); | ||
| } else if (field.selector) { | ||
| hasSelector.push(field.key); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
Refactor the hasSelector computation for clarity and efficiency.
The new implementation of hasSelector now checks for nested fields, which is a significant improvement in functionality. However, the current implementation can be optimized for clarity and efficiency. Consider using a recursive function to handle nested fields, which would make the code cleaner and potentially more efficient by avoiding repeated type checks and conversions.
Here's a suggested refactor:
- const hasSelector: string[] = [];
- fields.forEach((field) => {
- if ((field as any).fields) {
- Object.entries((field as any).fields).forEach(([key, subField]) => {
- if ((subField as any).selector) {
- hasSelector.push(key);
- }
- });
- } else if (field.selector) {
- hasSelector.push(field.key);
- }
- });
+ const hasSelector = fields.reduce((acc, field) => {
+ if (field.fields) {
+ acc.push(...Object.keys(field.fields).filter(key => field.fields[key].selector));
+ } else if (field.selector) {
+ acc.push(field.key);
+ }
+ return acc;
+ }, []);This refactor uses reduce to accumulate keys, simplifying the logic and potentially improving performance by minimizing the scope of checks and transformations.
Committable suggestion was skipped due to low confidence.
|
Thank you for merging my post with this one. This lets me know that awareness is brought to this. Not a big issue, just annoying. Thank you again. |
|
This is working now with 2024.10 update. Thank you so much. You guys/gals/them are amazing. |
Proposed change
Fix #21629.
hasSelectorneeds to also look into the subsections to find which sub-fields also have selectors.Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
hasSelectorproperty to detect selectors within nested fields, improving the accuracy of field representation.