Consolidate scroll buttons#112
Open
iamnoksio wants to merge 1 commit into
Open
Conversation
Comment on lines
174
to
195
| {overflow && ( | ||
| <button | ||
| type="button" | ||
| onClick={() => scrollTabs(-1)} | ||
| disabled={!canLeft} | ||
| aria-label="Scroll tabs left" | ||
| className={`${CHEVRON_BTN} mr-1`} | ||
| > | ||
| <ChevronLeft size={16} strokeWidth={2} aria-hidden="true" /> | ||
| </button> | ||
| <div className="flex items-center gap-0.5 mr-1 shrink-0"> | ||
| <button | ||
| type="button" | ||
| onClick={() => scrollTabs(-1)} | ||
| disabled={!canLeft} | ||
| aria-label="Scroll tabs left" | ||
| className={CHEVRON_BTN} | ||
| > | ||
| <ChevronLeft size={16} strokeWidth={2} aria-hidden="true" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={() => scrollTabs(1)} | ||
| disabled={!canRight} | ||
| aria-label="Scroll tabs right" | ||
| className={CHEVRON_BTN} | ||
| > | ||
| <ChevronRight size={16} strokeWidth={2} aria-hidden="true" /> | ||
| </button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Right-scroll affordance removed from right side
Placing both chevrons on the left removes the visual cue on the right side of the tab strip that additional tabs exist in that direction. With the previous layout, a visible (or disabled) right-arrow at the far right was the natural signal that there was more content. Users who scroll to the far right may not realise there's a ChevronRight button available, since it is now hidden behind the visible tabs on the left. Consider whether the consolidated placement still communicates intent sufficiently — adding a right-side fade/gradient mask is one common alternative used by browser UIs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/layout/UnifiedTabBar.tsx
Line: 174-195
Comment:
**Right-scroll affordance removed from right side**
Placing both chevrons on the left removes the visual cue on the right side of the tab strip that additional tabs exist in that direction. With the previous layout, a visible (or disabled) right-arrow at the far right was the natural signal that there was more content. Users who scroll to the far right may not realise there's a `ChevronRight` button available, since it is now hidden behind the visible tabs on the left. Consider whether the consolidated placement still communicates intent sufficiently — adding a right-side fade/gradient mask is one common alternative used by browser UIs.
How can I resolve this? If you propose a fix, please make it concise.
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
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.
Greptile Summary
This PR consolidates the two tab-scroll chevron buttons — previously one on each side of the tab strip — into a single grouped container placed on the left of the tab bar. It also reformats several multi-line expressions and function signatures for readability.
ChevronLeftandChevronRightbuttons are now rendered together in aflexwrapper on the left side of the tab bar, with the right-scroll button no longer occupying its own slot at the far right of the strip.Confidence Score: 4/5
Safe to merge — changes are limited to button layout and code formatting with no logic mutations.
The functional logic for scrolling, overflow detection, and tab management is untouched. The only behavioral change is moving the right-scroll chevron from the far right of the tab strip to a consolidated group on the left. This removes the right-side visual affordance that signalled more tabs exist in that direction — a UX trade-off worth team discussion before merging. A stale comment also remains that describes the old two-slot layout.
Files Needing Attention: src/components/layout/UnifiedTabBar.tsx — specifically the consolidated button group and the now-inaccurate comment block around line 79.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[UnifiedTabBar renders] --> B{overflow?} B -- No --> C[Tab strip fills full width\nNo scroll buttons shown] B -- Yes --> D["Render consolidated button group on LEFT side\nChevronLeft + ChevronRight"] D --> E{canLeft?} D --> F{canRight?} E -- No --> G[Left button disabled] E -- Yes --> H[Left button enabled] F -- No --> I[Right button disabled] F -- Yes --> J[Right button enabled] D --> K[Tab strip fills remaining right-side width] H --> L["scrollTabs scroll left"] J --> M["scrollTabs scroll right"]Comments Outside Diff (1)
src/components/layout/UnifiedTabBar.tsx, line 79-82 (link)The comment still says "Both slots render whenever there's overflow", but the new layout has a single consolidated group rather than two separate slots. The comment should be updated to describe the new structure, e.g. noting that both buttons are grouped together on the left and individually disabled when their respective edge is reached.
Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Consolidate scroll buttons" | Re-trigger Greptile