Repository navigation
fix(tags): make the tag filter's search find what it shows - #8692
talissoncosta wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true📝 WalkthroughWalkthroughThe tag filter trims and lowercases search text before matching tag labels. The archived and untagged rows now appear only when their labels match the search. The “No tags” message is hidden when either special row is visible. The untagged row uses a named empty-string ID. The special-row and tag-list Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Severity of issue fixed: Low Merge Risk: 🔵 Low · up to Search matching has no confirmed defect, but the extra prop should be removed to keep the component type-correct.
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 |
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #21303 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #21303 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #21303 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #21303 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
286eb5d2-4edc-4b1f-9977-114824431044
📒 Files selected for processing (1)
frontend/web/components/tables/TableTagFilter.tsx
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| className='px-2 py-2 mr-1' | ||
| tag={tag} | ||
| /> | ||
| <Tag key={tag.id} isDot tag={tag} /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Check whether TagContent accepts a disabled prop.
sed -n 1,20p frontend/web/components/tags/TagContent.tsx
rg -n 'TagContent[^a-zA-Z]' frontend/web --type=tsx -g '!**/TagContent.tsx' | rg 'disabled'Repository: Flagsmith/flagsmith
Length of output: 934
🏁 Script executed:
git diff --unified=8 76c480252978b3a90b90fd18c513bcfa10093589 062e0860251d6cdb3df5c900767a282cf796b8a5 -- frontend/web/components/tables/TableTagFilter.tsx frontend/web/components/tags/TagContent.tsx
printf '\n--- TagContent at reviewed head ---\n'
git show 062e0860251d6cdb3df5c900767a282cf796b8a5:frontend/web/components/tags/TagContent.tsx | nl -ba | sed -n '1,22p;96,140p'
printf '\n--- TableTagFilter call sites at reviewed head ---\n'
git show 062e0860251d6cdb3df5c900767a282cf796b8a5:frontend/web/components/tables/TableTagFilter.tsx | nl -ba | sed -n '160,200p'Repository: Flagsmith/flagsmith
Length of output: 10839
Remove the unsupported disabled prop from TagContent.
The call passes disabled, but TagContent declares only tag in its props type. The component derives disabled from tag itself, so the extra prop is redundant and causes an excess-property type error.
Suggested fix
-<TagContent disabled={Utils.tagDisabled(tag)} tag={tag} />
+<TagContent tag={tag} />
Visual Regression20 screenshots compared. See report for details. |
062e086 to
0cba330
Compare
0cba330 to
979ae88
Compare
979ae88 to
23f7d86
Compare
The table filter lowercased each tag's label but not what was typed, so searching for a tag by the capitalised name it displays under found nothing. The term is lowercased and trimmed once instead. The archived and untagged rows are not tags and were rendered whatever was typed, so a search that matched neither still showed them above an empty list, or beside a "No tags" message. They match the search now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
23f7d86 to
20e5a68
Compare
docs/if required so people know about the feature.Changes
Closes #8691
The filter lowercased each tag's label but not what was typed, so a tag displayed as "Onboarding" could only be found by typing
onboarding. Typed as it appears, it returned nothing. The term is lowercased and trimmed once instead, so a trailing space no longer empties the list either.The
archivedanduntaggedrows are not tags and rendered whatever was typed, so a search matching neither still showed them above an empty list, or beside a "No tags" message. They match the search now, and the empty state accounts for them.Also drops
flagGatedTags?.filter((tag) => tag), which filtered an array by the truthiness of its own elements.Found while working on #8465 and pulled out of #8689, which wants to stay a colour change. The two touch different parts of this file and rebase cleanly either way, so the merge order does not matter.
How did you test this code?
The match predicate is extracted to
tagFilterSearch.tsand covered by 12 cases in__tests__/tagFilterSearch.test.ts, including the capital that caused the bug, a trailing space, and an active filter surviving a search that does not match it.Typecheck at main's baseline, 890 errors both sides.
Manually, on the features list with at least one tag whose name starts with a capital:
zzz. The list is empty, "No tags" shows, and the archived and untagged rows are gone.zzzagain. The archived row stays, because an active filter you cannot see is a filter you cannot turn off.arch. Only the archived row shows.