fix/multifilter#891
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughFilterPanel now scopes add-filter queries to the component and adds ShadowRoot-aware ownership checks so clicking a filter button in one revo-grid closes another grid’s panel; isFilterBtn typing was generalized; an e2e test mounts two grids to verify single-panel behavior and dialog-position tolerances were relaxed. ChangesMulti-Grid Filter Panel Isolation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93cba6da73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/plugins/filter/filter.panel.tsx (1)
663-678: 💤 Low valueRemove unnecessary type assertion.
After the
instanceof Elementcheck on line 664, TypeScript already narrowstargettoElement. The explicit cast on line 668 is redundant.Suggested fix
private isOwnFilterButton(target: EventTarget | null) { if (!(target instanceof Element)) { return false; } - const element = target as Element; - - if (!isFilterBtn(element)) { + if (!isFilterBtn(target)) { return false; } const panelGrid = this.getOwningGrid(this.element); - const targetGrid = this.getOwningGrid(element); + const targetGrid = this.getOwningGrid(target); return !!panelGrid && panelGrid === targetGrid; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugins/filter/filter.panel.tsx` around lines 663 - 678, In isOwnFilterButton, remove the redundant type assertion "const element = target as Element" because the preceding "if (!(target instanceof Element))" already narrows target to Element; instead use the narrowed variable directly (e.g., rename or reuse "target" as the Element) when calling isFilterBtn, getOwningGrid, and comparing panelGrid/targetGrid in the isOwnFilterButton method to eliminate the unnecessary cast.Source: Linters/SAST tools
e2e/filtering.spec.ts (1)
124-134: 💤 Low valueConsider using the
withHeaderTestIdhelper for consistency.The column configuration uses
columnProperties: () => ({ 'data-testid': ... })directly, while other tests in this file use thewithHeaderTestIdhelper. Using the helper would improve consistency.♻️ Proposed refactor
{ prop: 'role', name: 'Role', filter: true, - columnProperties: () => ({ 'data-testid': `${prefix}-filter-role` }), + ...withHeaderTestId(`${prefix}-filter-role`), },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/filtering.spec.ts` around lines 124 - 134, Replace the inline columnProperties for the "role" column with the shared helper: use withHeaderTestId to attach the header test id instead of columnProperties: () => ({ 'data-testid': `${prefix}-filter-role` }); update the grid.columns entry for the column with prop 'role' (and keep filter: true and name: 'Role') to call withHeaderTestId(prefix, 'filter-role') (or the existing helper signature) so the test id pattern and consistency match other tests that use withHeaderTestId.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/filtering.spec.ts`:
- Around line 124-134: Replace the inline columnProperties for the "role" column
with the shared helper: use withHeaderTestId to attach the header test id
instead of columnProperties: () => ({ 'data-testid': `${prefix}-filter-role` });
update the grid.columns entry for the column with prop 'role' (and keep filter:
true and name: 'Role') to call withHeaderTestId(prefix, 'filter-role') (or the
existing helper signature) so the test id pattern and consistency match other
tests that use withHeaderTestId.
In `@src/plugins/filter/filter.panel.tsx`:
- Around line 663-678: In isOwnFilterButton, remove the redundant type assertion
"const element = target as Element" because the preceding "if (!(target
instanceof Element))" already narrows target to Element; instead use the
narrowed variable directly (e.g., rename or reuse "target" as the Element) when
calling isFilterBtn, getOwningGrid, and comparing panelGrid/targetGrid in the
isOwnFilterButton method to eliminate the unnecessary cast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d0d20c13-a749-48d2-aa67-1b73a69d2a8c
📒 Files selected for processing (3)
e2e/filtering.spec.tssrc/plugins/filter/filter.button.tsxsrc/plugins/filter/filter.panel.tsx
✅ Files skipped from review due to trivial changes (1)
- src/plugins/filter/filter.button.tsx
|
2 participants
Summary by CodeRabbit
Bug Fixes
Tests