-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
Preserve integration category filter on noop keyup #36844
base: current
Are you sure you want to change the base?
Preserve integration category filter on noop keyup #36844
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThe pull request introduces modifications to the integrations page, specifically targeting the logic for counting components by category and enhancing the search functionality. The changes include the removal of a line that prematurely ended a conditional block, allowing for accurate increments of the Changes
Sequence DiagramsequenceDiagram
participant User
participant SearchInput
participant FilterLogic
participant CategoryDisplay
User->>SearchInput: Enter search text
SearchInput->>FilterLogic: Check if text changed
alt Text is different from lastSearchText
FilterLogic->>CategoryDisplay: Apply search filter
FilterLogic->>SearchInput: Update lastSearchText
else Text is same
FilterLogic->>CategoryDisplay: No action
end
The sequence diagram illustrates the enhanced search filtering process, showing how the system now checks if the search text has changed before applying filters, which helps optimize performance and reduce unnecessary updates. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (4)
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 (
|
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
🧹 Nitpick comments (2)
source/integrations/index.html (2)
312-324
: LGTM! Efficient search text tracking implementation.Good implementation that:
- Prevents unnecessary filter updates
- Properly sanitizes input
- Handles empty search appropriately
Minor suggestion: Consider using strict equality operator (
!==
) for consistency with the rest of the codebase.- if (lastSearchText != text) { + if (lastSearchText !== text) {
Line range hint
211-324
: Overall Assessment
- ✅ The main changes successfully fix the category filter preservation issue by tracking search text state.
- ❓ The search filter modification on line 211 needs review as it might cause unexpected behavior.
- 💡 Minor style improvement suggested for consistency.
The implementation effectively solves the reported issue, but please review the search filter logic change as it seems unrelated to the PR's objective.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/integrations/index.html
(4 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 (1)
source/integrations/index.html (1)
70-70
: LGTM! Code formatting improvements.The changes maintain correct category counting logic while improving code readability.
Also applies to: 101-101
filter = comp => | ||
comp.search.indexOf(search) !== -1 || | ||
comp.cat.find((c) => c.includes("#")) != undefined; |
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.
💡 Codebase verification
Remove unrelated category filter condition
The condition comp.cat.find((c) => c.includes("#")) != undefined
should be removed as:
- There's no evidence of '#' being used in category names in the codebase
- It would include components in search results regardless of the search term
- It deviates from the PR's purpose of preserving category filters
🔗 Analysis chain
Review search filter logic modification.
The added condition comp.cat.find((c) => c.includes("#")) != undefined
will include components that have '#' in their categories, regardless of the search term. This seems unrelated to the PR's objective and could cause unexpected search results.
Let's verify the impact:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for categories containing '#' character that might affect search results
ast-grep --pattern 'cat: [$$$]' | grep -o '"[^"]*#[^"]*"'
Length of output: 59
Script:
#!/bin/bash
# Find component definitions and category patterns
echo "=== Looking for component definitions ==="
rg -l "components?\s*=\s*\[" --type html --type js
echo -e "\n=== Checking category-related code ==="
rg "cat\s*[:=]" -A 2 --type html --type js
echo -e "\n=== Looking for '#' character usage ==="
rg "#['\"]?\s*[,\]]" --type html --type js
Length of output: 4883
1ab0826
to
b3dfc05
Compare
Proposed change
If you follow a link to an integration category, e.g. https://www.home-assistant.io/integrations/#irrigation, press ctrl+tab, and revisit the tab, the category filter is lost. This is because the search box is auto-focused, and any keyup triggers an update. Instead, track the actual value used in the box and only update if it changes.
Also a couple of trailing space style fixes snuck in.
Type of change
(technically none of these, but by spirit of PR I'm calling this a readability improvement)
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit
Bug Fixes
Performance