Repository navigation
Conversation
There was a problem hiding this comment.
I couldn't check this in a running app: the side panel tabs are only in the desktop/web app, and this environment has no browser. Could you attach before/after screenshots (or a short video) of the real app, with the strip scrolled partway and the narrow-panel case from #60233?
From reading the code I don't think this does what it describes. The mask is on the whole strip, so the new layer keeps a fixed 28px band before the "+" visible whatever is under it. It doesn't follow any tab's close button. Once the strip scrolls (all tabs at their 72px minimum since #60244), that band will often show part of a label or the gap between tabs. A close button that lands in the fade, now just 28px further left, is still faded but clickable. And while there are more tabs to scroll to, content fades out and then pops back at full opacity in the band, which breaks the fade that signals more tabs (see inline).
If the goal is that a faded control can't be clicked, a fix that tracks tab positions would hold up better than a fixed band. For example, the scroll handler could keep the active or last tab clear of the fade, or controls under the fade could stop taking clicks.
Public API: unchanged (CSS only, one internal custom property --tab-strip-close).
| width of the sticky "+", which the second layer keeps unmasked. The end fade stops short of a tab's trailing close | ||
| button (20px, plus the wrapper's 4px end padding), which the third layer keeps unmasked too: the fade must never | ||
| cover a control it leaves clickable. */ | ||
| --tab-strip-close: 28px; |
There was a problem hiding this comment.
This band sits at a fixed spot in the strip (the last 28px before the "+"), not on a tab, so it only covers a close button when the strip happens to stop with a tab's end right there. When the strip is scrolled to the middle (fade-end > 0), the first layer fades to transparent at 100% - add - 28px and this layer is opaque right after. Content fades out, then reappears at full strength in a 28px slice: a hard edge that weakens the 'more tabs this way' cue. Close buttons that land in the fade, now 28px further left, are still faded but clickable. Also, 20px button + 4px end padding is 24px, not 28px.
There was a problem hiding this comment.
All three are fair. The 20+4 sum was wrong, and the comment read like the whole 28px was button. I pushed a follow-up commit: the band is now 32px (24px button plus the 8px gap in front of it), so a parked close button is fully covered wherever the wheel stops — the strip often parks with part of the last button in the fade, since the wheel scrolls in 50px steps, and the issue's screenshots are exactly that state.
The tradeoff you caught is real: a tab passing through now pops back to full opacity over that last 32px before the "+". If that sliver looks too visible to you, I can drop the third layer and track the parked-button case in an issue instead. And no other button can reach the band: tabs floor at 72px with an 8px gap, so the next button sits about 88px clear.
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
There was a problem hiding this comment.
My earlier concern is still open. The new commit widens the always-visible band from 28px to 32px but doesn't change how it works: the band is still at a fixed spot in the strip, not on a tab. When the strip is scrolled all the way to the end, the end fade is 0 and nothing is faded, and that is the only position where the last tab's close button and the 8px gap sit exactly in the band. While there are more tabs to scroll to, the band still shows whatever part of a tab is under it, with a hard edge after the fade, and a close button in the fade (now 32px further left) is still faded but clickable.
I still couldn't run this (desktop/web UI, no browser here). Screenshots of the real app with the strip scrolled partway would help.
Public API: unchanged (CSS only).
| width of the sticky "+", which the second layer keeps unmasked. */ | ||
| width of the sticky "+", which the second layer keeps unmasked. The end fade stops short of a tab's trailing close | ||
| button (24px: a 20px button plus the wrapper's 4px end padding), which the third layer keeps unmasked together with | ||
| the 8px gap in front of it, so the button stays crisp wherever the strip parks. The fade must never cover a control |
There was a problem hiding this comment.
This comment says the button "stays crisp wherever the strip parks", but the band is at a fixed spot in the strip. It only lines up with a close button when the strip is scrolled all the way to the end, and there the end fade is already 0. At any other scroll position, a close button inside the fade is still faded but clickable.
Issue for this PR
Closes #60233
Type of change
What does this PR do?
The end fade in the side panel tab strip sat over the close button, so on
narrow panels the button looked invisible but still reacted to clicks (masks
don't block pointer events). I added a third mask layer that keeps the
trailing 32px (the 24px close button plus the 8px gap before it) unmasked, the same way the
sticky "+" was already kept unmasked. The label fade still works as before.
How did you verify your code works?
Reproduced in a static DOM harness on Windows 11: with the current CSS the
close button fades out but stays clickable; with the change it renders while
the label fade stays in place. prettier and typecheck on packages/ui pass.
Screenshots / recordings
Checklist