ProfileList - Add active profile indicator#804
Open
Sartoric wants to merge 1 commit into
Open
Conversation
…tility tabs - Added visual indicators for selected profile to the "extraction" and "utility" tabs.
Contributor
📝 WalkthroughWalkthroughIn ChangesUtility Profile Active Tab Indicator
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@web/src/pages/ProfileList/index.jsx`:
- Around line 661-662: The active-tab logic in ProfileList/index.jsx is using
isUtilityProfileActive as a proxy for the visible selected tab, which causes
Extraction to appear active when the selected profile is hidden by search.
Update the selection derivation in the ProfileList component to compute the
visible selected profile explicitly, or split the state into separate extraction
and utility booleans, and use that result where the tab active state is
determined. Keep the logic aligned with the profilesToShow filtering so only
visible selections drive the UI.
- Around line 1004-1019: The tab indicator dots in ProfileList are currently
rendered as separate children of the tablist, which breaks the intended tab
semantics. Update the tab rendering around the Extraction and Utility `<button
role='tab'>` elements so each decorative dot is moved inside the corresponding
tab button and marked `aria-hidden='true'`, keeping the indicator associated
with the active tab while preserving accessible tablist structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7584675-35da-42f5-aac1-83d1299a6015
📒 Files selected for processing (1)
web/src/pages/ProfileList/index.jsx
Comment on lines
+661
to
+662
| const isUtilityProfileActive = profilesToShow.filter(p => p.utility && p.selected).length > 0; | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Derive the visible selected tab explicitly.
Line 661 only answers “is a visible utility profile selected?”. The Extraction dot then falls back to !isUtilityProfileActive, which also becomes true when the current selected profile is hidden by search, so the UI can show Extraction as active even when no visible profile is selected. Derive the visible selected profile (or separate extraction/utility booleans) instead of negating the utility flag.
Suggested fix
- const isUtilityProfileActive = profilesToShow.filter(p => p.utility && p.selected).length > 0;
+ const selectedVisibleProfile = profilesToShow.find(p => p.selected);
+ const isUtilityProfileActive = !!selectedVisibleProfile?.utility;
+ const isExtractionProfileActive = !!selectedVisibleProfile && !selectedVisibleProfile.utility;🤖 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 `@web/src/pages/ProfileList/index.jsx` around lines 661 - 662, The active-tab
logic in ProfileList/index.jsx is using isUtilityProfileActive as a proxy for
the visible selected tab, which causes Extraction to appear active when the
selected profile is hidden by search. Update the selection derivation in the
ProfileList component to compute the visible selected profile explicitly, or
split the state into separate extraction and utility booleans, and use that
result where the tab active state is determined. Keep the logic aligned with the
profilesToShow filtering so only visible selections drive the UI.
Comment on lines
+1004
to
+1019
| className={`tab ${activeTab === 'extraction' ? 'tab-active' : ''} ${!isUtilityProfileActive ? 'pr-[5px]' : ''}`} | ||
| onClick={() => setActiveTab('extraction')} | ||
| aria-label='Switch to extraction tab' | ||
| > | ||
| Extraction | ||
| </button> | ||
| {!isUtilityProfileActive && <div class='bg-primary mr-[7px] h-2 w-2 rounded-full' />} | ||
| <button | ||
| role='tab' | ||
| className={`tab ${activeTab === 'utility' ? 'tab-active' : ''}`} | ||
| className={`tab ${activeTab === 'utility' ? 'tab-active' : ''} ${isUtilityProfileActive ? 'pr-[5px]' : ''}`} | ||
| onClick={() => setActiveTab('utility')} | ||
| aria-label='Switch to utility tab' | ||
| > | ||
| Utility | ||
| </button> | ||
| {isUtilityProfileActive && <div class='bg-primary mr-[7px] h-2 w-2 rounded-full' />} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the indicator inside the tab button.
The new dot elements are direct children of role='tablist' even though they are decorative, not tabs. That muddies the tablist semantics for assistive tech. Render the dot inside the corresponding <button role='tab'> and mark it aria-hidden='true'.
Suggested fix
- <button
+ <button
role='tab'
- className={`tab ${activeTab === 'extraction' ? 'tab-active' : ''} ${!isUtilityProfileActive ? 'pr-[5px]' : ''}`}
+ className={`tab ${activeTab === 'extraction' ? 'tab-active' : ''}`}
onClick={() => setActiveTab('extraction')}
aria-label='Switch to extraction tab'
>
Extraction
+ {isExtractionProfileActive && (
+ <span aria-hidden='true' className='bg-primary ml-2 h-2 w-2 rounded-full' />
+ )}
</button>
- {!isUtilityProfileActive && <div class='bg-primary mr-[7px] h-2 w-2 rounded-full' />}
<button
role='tab'
- className={`tab ${activeTab === 'utility' ? 'tab-active' : ''} ${isUtilityProfileActive ? 'pr-[5px]' : ''}`}
+ className={`tab ${activeTab === 'utility' ? 'tab-active' : ''}`}
onClick={() => setActiveTab('utility')}
aria-label='Switch to utility tab'
>
Utility
+ {isUtilityProfileActive && (
+ <span aria-hidden='true' className='bg-primary ml-2 h-2 w-2 rounded-full' />
+ )}
</button>
- {isUtilityProfileActive && <div class='bg-primary mr-[7px] h-2 w-2 rounded-full' />}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| className={`tab ${activeTab === 'extraction' ? 'tab-active' : ''} ${!isUtilityProfileActive ? 'pr-[5px]' : ''}`} | |
| onClick={() => setActiveTab('extraction')} | |
| aria-label='Switch to extraction tab' | |
| > | |
| Extraction | |
| </button> | |
| {!isUtilityProfileActive && <div class='bg-primary mr-[7px] h-2 w-2 rounded-full' />} | |
| <button | |
| role='tab' | |
| className={`tab ${activeTab === 'utility' ? 'tab-active' : ''}`} | |
| className={`tab ${activeTab === 'utility' ? 'tab-active' : ''} ${isUtilityProfileActive ? 'pr-[5px]' : ''}`} | |
| onClick={() => setActiveTab('utility')} | |
| aria-label='Switch to utility tab' | |
| > | |
| Utility | |
| </button> | |
| {isUtilityProfileActive && <div class='bg-primary mr-[7px] h-2 w-2 rounded-full' />} | |
| <button | |
| role='tab' | |
| className={`tab ${activeTab === 'extraction' ? 'tab-active' : ''}`} | |
| onClick={() => setActiveTab('extraction')} | |
| aria-label='Switch to extraction tab' | |
| > | |
| Extraction | |
| {isExtractionProfileActive && ( | |
| <span aria-hidden='true' className='bg-primary ml-2 h-2 w-2 rounded-full' /> | |
| )} | |
| </button> | |
| <button | |
| role='tab' | |
| className={`tab ${activeTab === 'utility' ? 'tab-active' : ''}`} | |
| onClick={() => setActiveTab('utility')} | |
| aria-label='Switch to utility tab' | |
| > | |
| Utility | |
| {isUtilityProfileActive && ( | |
| <span aria-hidden='true' className='bg-primary ml-2 h-2 w-2 rounded-full' /> | |
| )} | |
| </button> |
🤖 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 `@web/src/pages/ProfileList/index.jsx` around lines 1004 - 1019, The tab
indicator dots in ProfileList are currently rendered as separate children of the
tablist, which breaks the intended tab semantics. Update the tab rendering
around the Extraction and Utility `<button role='tab'>` elements so each
decorative dot is moved inside the corresponding tab button and marked
`aria-hidden='true'`, keeping the indicator associated with the active tab while
preserving accessible tablist structure.
|
Can we have this integrated into the statistics page too? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update ProfileList: add active profile indicator for extraction and utility tabs
Summary by CodeRabbit
New Features
Bug Fixes