Add tab item cuttoff logic. #10410
Add tab item cuttoff logic. #10410
Conversation
| scrollContainer.style.columnGap = '2px'; | ||
| } | ||
|
|
||
| maybeCuttOffLastTabItem(); |
There was a problem hiding this comment.
Small diverging from IB, after experimenting on different screen sizes, I set the logic to run twice, double checking of previous gap worked, by first compressing the spacing, and if that didn't work, by making bigger gap, and in all cases either of these approaches will cause a cutoff in last item
Screen.Recording.2025-03-12.at.10.03.21.mov
|
Build files for a33f3b9 are ready:
|
| @@ -348,6 +416,10 @@ export default function ChipTabGroup( { allMetricItems, savedItemSlugs } ) { | |||
| // Reset the unstaged selection when selection panel is closed. | |||
| resetUnstagedSelection(); | |||
| } | |||
|
|
|||
| if ( ! isSelectionPanelOpenPrevious && isSelectionPanelOpen ) { | |||
| maybeCuttOffLastTabItem(); | |||
There was a problem hiding this comment.
Another diverging from IB, this had to be invoked when selection panel is opened, as on mount the panel is hidden and rectangle bound is returning 0 for all values
There was a problem hiding this comment.
Hey @zutigrm, a few small comments inline.
More broadly, from a UX perspective the difference between 2px and 20px is pretty big, in the original thread @sigal-teller said:
increase the space a little
Is it possible to achieve the same effect keeping the gap closer to 10px, perhaps adjusting between 10px and 20px as the 2px gap looks quite different to other margins within the app?
| @@ -356,16 +428,34 @@ export default function ChipTabGroup( { allMetricItems, savedItemSlugs } ) { | |||
| isMobileBreakpoint, | |||
| newlyDetectedMetricsKeys, | |||
| resetUnstagedSelection, | |||
| maybeCuttOffLastTabItem, | |||
There was a problem hiding this comment.
This typo was in the IB, it should be CutOff not CuttOff throughout the PR.
| visibleItems.push( index ); | ||
| } | ||
| } ); | ||
| const nextTabItem = tabItems[ visibleItems.length - 1 + 1 ]; |
There was a problem hiding this comment.
Couldn't this just be:
| const nextTabItem = tabItems[ visibleItems.length - 1 + 1 ]; | |
| const nextTabItem = tabItems[ visibleItems.length ]; |
| @@ -224,6 +237,61 @@ export default function ChipTabGroup( { allMetricItems, savedItemSlugs } ) { | |||
| ).getKeyMetricsConversionEventWidgets(); | |||
| } ); | |||
|
|
|||
| const maybeCuttOffLastTabItem = useCallback( () => { | |||
There was a problem hiding this comment.
I suggest you add a short code comment for this function to remind us in future why we added this logic.
Summary
Addresses issue:
Relevant technical choices
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist