Fix: Remove leading # from search result tags in advanced search#1115
Fix: Remove leading # from search result tags in advanced search#1115mdroidian wants to merge 1 commit into
Conversation
- Strip # prefix from node badges to show correct 3-letter abbreviations - Applies to both nodeConfig and nodeTypeLabel fallback cases - Fixes tags showing as #CL and #QU instead of CLM and QUE Co-authored-by: Michael Gartner <mdroidian@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@codex review |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| {nodeConfig ? getNodeBadgeText(nodeConfig) : result.nodeTypeLabel} | ||
| {nodeConfig | ||
| ? getNodeBadgeText(nodeConfig) | ||
| : result.nodeTypeLabel.replace(/^#/, "").slice(0, 3).toUpperCase()} |
There was a problem hiding this comment.
🟡 Duplicated badge-formatting logic inline instead of reusing a utility function
The badge text formatting logic .replace(/^#/, "").slice(0, 3).toUpperCase() at line 109 duplicates the same transformation already implemented in getNodeBadgeText (lines 52-55). This violates the AGENTS.md rule: "Prefer util functions for reusable logic and common operations" and "Prefer small, focused functions over inline code". A small helper (e.g., formatBadgeText(label: string): string) should be extracted and used in both getNodeBadgeText and the fallback branch.
Prompt for agents
The badge text formatting logic (strip leading #, take first 3 chars, uppercase) is duplicated: once in getNodeBadgeText (line 52-55) and once inline in the ResultRow fallback (line 109). Per AGENTS.md rules, reusable logic should be extracted into utility functions.
Suggested approach:
1. Extract a small helper like formatBadgeText(text: string): string that does text.replace(/^#/, '').slice(0, 3).toUpperCase()
2. Use it in getNodeBadgeText: return formatBadgeText(node.tag?.trim() || node.text)
3. Use it in the ResultRow fallback: formatBadgeText(result.nodeTypeLabel)
This keeps the logic in one place and follows the repo's code organization guidelines.
Was this helpful? React with 👍 or 👎 to provide feedback.
| {nodeConfig | ||
| ? getNodeBadgeText(nodeConfig) | ||
| : result.nodeTypeLabel.replace(/^#/, "").slice(0, 3).toUpperCase()} |
There was a problem hiding this comment.
🚩 Fallback branch now truncates nodeTypeLabel (behavioral change)
Previously when nodeConfig was undefined, the full result.nodeTypeLabel string was displayed in the <Tag>. Now it's truncated to 3 uppercase characters (line 109). This is likely an intentional fix for visual consistency with the getNodeBadgeText path, but it is a user-visible change: any result without a matching nodeConfig will now show an abbreviated badge instead of the full label.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
trangdoan982
left a comment
trangdoan982
left a comment
There was a problem hiding this comment.
@mdroidian the devin comments are worth addressing.
3 participants
Problem
In the advanced search dialog, node type tags were displaying with a leading
#character, showing abbreviated forms like#CLand#QUinstead of the intendedCLMandQUE.Solution
Updated the tag display logic to strip the leading
#character before extracting the 3-letter abbreviation:getNodeBadgeText()to remove#prefix using.replace(/^#/, "")before slicingnodeConfigis unavailableChanges
apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchDialog.tsxgetNodeBadgeTextfunction to strip leading#before taking first 3 charactersnodeTypeLabelfallback with same#stripping logicTesting
✅ Build: Compiled successfully with 0 errors
✅ Type checking: No TypeScript errors
✅ Linting: All style checks passed
✅ Logic verification: Tested with multiple scenarios
Test Cases Verified
#CLM#CL❌CLM✅#QUE#QU❌QUE✅#CLAIM#CL❌CLA✅CLAIMCLA✅CLA✅The fix ensures that:
#have the prefix removed before abbreviation#continue to work as expectedFixes ENG-1839
Linear Issue: ENG-1839