Open Bug 2062425 Opened 24 days ago Updated 1 day ago

Various issues with vertical split view tabs

Categories

(Firefox :: Tabbed Browser: Split View, defect, P2)

defect
Points:
3

Tracking

()

ASSIGNED

People

(Reporter: bvisness, Assigned: jsudiaman)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fidefe-nova])

Attachments

(14 files, 1 obsolete file)

110.52 KB, image/png
Details
499.83 KB, video/mp4
Details
2.06 MB, video/quicktime
Details
72.50 KB, image/png
Details
134.65 KB, image/png
Details
39.25 KB, image/png
Details
4.74 MB, video/quicktime
Details
4.03 MB, video/quicktime
Details
73.11 KB, image/png
Details
25.55 KB, image/png
Details
802.47 KB, image/png
Details
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

After some conversation with kcochrane, I am just opening up one big bug that describes lots of issues with split view tabs when using vertical tabs. All of these issues were reproduced on Mac with build 20260810154837. Some issues only reproduce on my Mac's retina display, while others only reproduce on my external monitor. (The only difference, I believe, is the DPI.)

My description will summarize all the bugs and then I will follow up with screenshots. None of the issues need steps to reproduce beyond the obvious.

Major issues:

  1. The background of split view tabs is too narrow when a scrollbar is present. (All DPIs, both standard and compact.)
  2. Split view tabs are too wide when tabs are pinned. (All DPIs, compact only.)
  3. The background of split view tabs varies awkwardly in border-radius when resizing the sidebar. (All DPIs, both standard and compact.)

Additional small alignment issues:

  1. The background of collapsed split view tabs is 1px too wide. (Normal DPI only, standard only.)
  2. The contents of collapsed split view tabs are not quite centered. (High-DPI only, standard only.)
  3. Individual tabs are not vertically centered in their split container. (All DPIs, both standard and compact.)

For these, I have a hunch that something is specified as an odd number of pixels, which is then causing bad centering in various places.

And one final pet peeve that seems to apply everywhere and therefore might be part of the design spec but nonetheless makes me sad:

  1. The radius and padding around split tabs is inconsistent, with varying thickness of border around a tab.
Attached image scrollbar-problems.png

1. Collapsed split view tab backgrounds interact badly with scrollbars

When the sidebar needs to scroll vertically, the scrollbar interacts badly with collapsed split view tabs. The background gets shoved to the side and is obviously too narrow to contain its tabs. This applies to both standard and compact densities.

I should say -- scrollbars generally seem to cause lots of other problems too, like chopping off tab groups and causing a small amount of horizontal scroll in some cases. Just gross all around.

Attached video pin-unpin.mp4

2. Collapsed split view tabs are too wide when tabs are pinned in compact mode

When tabs are pinned, and the sidebar is collapsed, and compact mode is enabled, split view tabs become too wide. Standard mode seems unaffected, and everything seems fine when the sidebar is not collapsed (at a variety of widths).

3. The border-radius of split view tabs varies while resizing

The background of a split view tab varies in border-radius while resizing, specifically when dragging the sidebar out of the collapsed state or when animating the expand/collapse of the sidebar. This causes the background to touch the tabs inside and is generally very ugly. It seems like the radius should just be fixed to a known value so this does not happen.

The Bugbug bot thinks this bug should belong to the 'Firefox::Tabbed Browser: Split View' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: General → Tabbed Browser: Split View

4 and 5. Bad centering of collapsed tabs in standard density

For a collapsed split view tab, depending on whether you view the tab on a high-DPI or normal-DPI monitor, either the background is centered but the content is not, or the content is centered but the background is not.

Attached image vcenter.png

6. Uncollapsed tabs not vertically centered in their container

In all modes and for all DPIs, when the sidebar is not collapsed and is sufficiently wide to display the tabs side by side, the tabs are not vertically centered within their containers. This may be due to the sizing of the background rather than the sizing of the tabs themselves, however.

Attached image border-sadness.png

7. Inconsistent borders everywhere

Across all uses of split view tabs in Nova, whether vertical or horizontal, compact or standard, the widths of the rounded borders around split view tabs are inconsistent. Generally, there is a pixel of extra padding on the sides that is not there on the top. This makes me sad because the border around the tab is therefore not a consistent thickness.

Priority: -- → P2
Whiteboard: [fidefe-post-nova]

Thank you for the detailed report!

Some of these issues might be better to fix together in one patch if they're highly related, while others may be better to fix in standalone patches. Can use judgment to determine which makes sense.

Severity: -- → S3
Points: --- → 1
Whiteboard: [fidefe-post-nova] → [fidefe-nova]
Duplicate of this bug: 2061196
Assignee: nobody → jsudiaman
Status: NEW → ASSIGNED

Thanks for the report. I took a look at some of your mentioned issues, and I wonder if they are still a problem in the latest Nightly build. Would you mind double checking, please?


(In reply to Ben Visness [:bvisness] from comment #1)

Created attachment 9625003 [details]
scrollbar-problems.png

1. Collapsed split view tab backgrounds interact badly with scrollbars

When the sidebar needs to scroll vertically, the scrollbar interacts badly with collapsed split view tabs. The background gets shoved to the side and is obviously too narrow to contain its tabs. This applies to both standard and compact densities.

I should say -- scrollbars generally seem to cause lots of other problems too, like chopping off tab groups and causing a small amount of horizontal scroll in some cases. Just gross all around.

I could not reproduce this behavior.


(In reply to Ben Visness [:bvisness] from comment #5)

Created attachment 9625015 [details]
collapsed-centering-standard.png

4 and 5. Bad centering of collapsed tabs in standard density

For a collapsed split view tab, depending on whether you view the tab on a high-DPI or normal-DPI monitor, either the background is centered but the content is not, or the content is centered but the background is not.

The margins and padding – per DevTools – look uniform to me. I'm not entirely sure how you're getting the measurements in your screenshot, or exactly what you're measuring. Some details about your verification process would be helpful!


(In reply to Ben Visness [:bvisness] from comment #6)

Created attachment 9625017 [details]
vcenter.png

6. Uncollapsed tabs not vertically centered in their container

In all modes and for all DPIs, when the sidebar is not collapsed and is sufficiently wide to display the tabs side by side, the tabs are not vertically centered within their containers. This may be due to the sizing of the background rather than the sizing of the tabs themselves, however.

Same deal here, it seems to be padded evenly.

Flags: needinfo?(bvisness)

Yes, issue 1 is still happening. You have to set scrollbars to always show in macOS settings, or have a mouse plugged in so that scrollbars always show. If scrollbars only appear while actively scrolling, this will not occur.

For 4 and 5, my "verification process" is to look at it with my human eyeballs and notice that things are not centered. For each of my screenshots, I literally just zoomed in in an image editor and counted the pixels.

I know that dev tools says that the margins and paddings are equal, but if you look in the dev tools you find that the sizing values are fractional, and therefore need to get rounded to the nearest integer pixel value when rendering.

Specifically, the split view wrapper in standard mode is 38px wide, but has margin of 5.5px and padding of 2.75px on each side, leaving a content area of 32.5px. Now how is any monitor, even a high-DPI one, supposed to handle padding of 2.75px? On my Macbook's display, which is double the resolution of CSS pixels, you are still attempting to set a padding of 5.5 screen pixels on each side. If you zoom in on my attachment in any image editor (including Preview on Mac) and count the pixels, you will see that the padding is rounding to 6px on the left and 5px on the right, because the rounding has gone in opposite directions. Therefore, the border between the two split view tabs is off-center.

Furthermore, on a standard-DPI monitor, where screen pixels = CSS pixels, what is supposed to happen with a 32.5px tab? Something has to give. Nova's sizing and spacing values should be more carefully chosen to ensure that you get integer pixel values for everything.

This same exact issue is also to blame for 6. Padding of 2.75px is just not a good idea. Again in this screenshot you can easily verify that it is rounding to 5 screen pixels on top and 6 screen pixels on bottom.

Flags: needinfo?(bvisness)

#pinned-tabs-container was mistakenly using --tab-pinned-min-width-expanded to size its items, even for the non-expanded case.

When both sides of a split view's margin or padding are subpixel values, there is no guarantee that both sides will round to the same value when painted on-screen.

To avoid such a scenario, (e.g. having 6px of margin on the left, and 5px on the right), round off the spacing calculations before passing them off to their respective CSS properties.

Removed margins from .tab-background, as these caused horizontal paddings to grow longer than the vertical paddings.

Attachment #9632763 - Attachment is obsolete: true
See Also: → 2050444
Points: 1 → 3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: