Closed Bug 1755036 Opened 2 years ago Closed 2 years ago

Improve the pane-splitters for the 3pane and to use elsewhere

Categories

(Thunderbird :: General, task)

Tracking

(thunderbird_esr91 unaffected)

RESOLVED FIXED
99 Branch
Tracking Status
thunderbird_esr91 --- unaffected

People

(Reporter: henry-x, Assigned: henry-x)

References

(Blocks 1 open bug)

Details

Attachments

(11 files, 2 obsolete files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

I was working on the pane splitter for bug 1720348 and I noticed a few things to change before hand, and ways to improve their usage in the new about:3pane.

Summary: Improve the use of pane-splitters for the 3pane and to use elsewhere → Improve the pane-splitters for the 3pane and to use elsewhere

Also rename the "resize" attribute to "resize-sibling" to be more meaningful and not overlap with the new "resize-direction" attribute.

Assignee: nobody → henry
Status: NEW → ASSIGNED

Also, make sure splitters are always visible whilst resizing. The splitter isn't necessarily under the mouse when resizing, so we can't rely on :hover.

Depends on D138558

We assign each child to its own named grid-area, and we simple adjust the grid-template for each of the four layouts.

Depends on D138559

The new layout prevents resizing the panes larger than the body, and works well when resizing the window.

We also give the thread pane a min-width to stop it collapsing.

We also give take into account that a splitter can only consume space from the opposite sibling, not from the full parent. This is most relevant in the Vertical layout.

Depends on D138560

I'm adding a generic review here since there are a lot of patches and I can't exactly pin point which causes which.

  • When viewing the account central, the center pane is not properly resizable, and if the Folder Pane was previously resized to a larger width when viewing the message thread page, it snaps back to its min-width immediately after switching to the account central and interacting with the splitter.
  • The splitter shouldn't have an hover/active effect.
  • The splitter should have an hover effect only if the resizable pane is collapsed (like the folder pane works right now).
  • The pane should collapse as soon as it reaches its min-width when the splitter is dragged. Right now, there's a small threshold that prevents a full collapse unless the drag action surpasses the min-width by a few pixels. I see the reasons why it was done, but it feels a bit janky and kinda like a mistake.
  • I'm not sure if it's possible, but the various hover/focus/scroll/etc mouse event related effects should be disabled when the splitter is active and a resize is in progress. If you try to resize quickly, the mouse causes hover effects on the scrollbar, or a folder, or a message row, and it looks a bit jarring. You can actually interact with a pane while resizing if you accidentally scroll. This is an edge case but we should definitely prevent it.

Great work nonetheless, and thanks for bringing all these great improvements.

Attachment #9263526 - Attachment description: Bug 1755036 - Improve the about3pane grid layout. r=aleca,darktrojan → Bug 1755036 - Make pane-splitter resizing relations explicit. r=aleca,darktrojan

The new layout prevents resizing the panes larger than the body. This works well when the window is resized.

Depends on D138561

Attachment #9263524 - Attachment is obsolete: true

(In reply to Alessandro Castellani [:aleca] from comment #5)

I'm adding a generic review here since there are a lot of patches and I can't exactly pin point which causes which.

Thanks for the feedback. It helped smooth some things out.

  • When viewing the account central, the center pane is not properly resizable, and if the Folder Pane was previously resized to a larger width when viewing the message thread page, it snaps back to its min-width immediately after switching to the account central and interacting with the splitter.

Thanks for spotting this. It was because the splitter was assuming that the next element siblings are what is resized. But in the account central display, the next element sibling is hidden, so has no size to share. I changed it so the relations are explicitly given.

  • The pane should collapse as soon as it reaches its min-width when the splitter is dragged. Right now, there's a small threshold that prevents a full collapse unless the drag action surpasses the min-width by a few pixels. I see the reasons why it was done, but it feels a bit janky and kinda like a mistake.
  • I'm not sure if it's possible, but the various hover/focus/scroll/etc mouse event related effects should be disabled when the splitter is active and a resize is in progress. If you try to resize quickly, the mouse causes hover effects on the scrollbar, or a folder, or a message row, and it looks a bit jarring. You can actually interact with a pane while resizing if you accidentally scroll. This is an edge case but we should definitely prevent it.

Both in a new patches. Good suggestion on the latter!

  • The splitter shouldn't have an hover/active effect.
  • The splitter should have an hover effect only if the resizable pane is collapsed (like the folder pane works right now).

Thanks, this is on the todo list.

The updates patches look great!

I'm seeing this error message when launching TB:
JavaScript error: chrome://messenger/content/pane-splitter.js, line 264: TypeError: can't access property "style", this.resizeElement is undefined

Flags: needinfo?(henry)

(In reply to Alessandro Castellani [:aleca] from comment #10)

I'm seeing this error message when launching TB:

Did you build? I was getting the same error for a while because about3Pane.xhtml was out of date. (I did build, but bug 1721311.)

(In reply to Geoff Lankow (:darktrojan) from comment #11)

Did you build? I was getting the same error for a while because about3Pane.xhtml was out of date. (I did build, but bug 1721311.)

Yes, I just did a full build and also clobbered. I'm still seeing it when this full stack of patches is applied.

Oh you're right, I am seeing that again. If you flip reorder the attributes in the XHTML file it appears to go away, but we shouldn't rely on attributes being in a particular order.

Attachment #9264043 - Attachment is obsolete: true

Whilst collapsed, we only show a resize arrow that points away from the collapsed element.

Depends on D138806

Updated the patches to address the error

Flags: needinfo?(henry)
Target Milestone: --- → 99 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/8c30a7a72d38
Make splitter resize-direction explicit. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/ac6f21cd7465
Simplify the grid CSS for the 3pane. r=aleca
https://hg.mozilla.org/comm-central/rev/eeb3facddeee
Make pane-splitter resizing relations explicit. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/a267b6eb72e3
Improve the about3pane grid layout when size is constrained. r=aleca
https://hg.mozilla.org/comm-central/rev/74ae7e70c27d
Prevent pointer events whilst resizing with a pane-splitter. r=aleca
https://hg.mozilla.org/comm-central/rev/eba4866d79e3
Change the pane-splitter cursor when collapsed. r=aleca

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/e3765d25ea00
Introduce standard styling for the html pane-splitter. r=aleca

We no longer require an oppositeElement to make sure we don't conflict with its min-width or min-height CSS properties. This can be handled by the layout styling.

We also add tests for when the resizeElement has a min-width, min-height, max-width or max-height CSS property.

We also renamed the "min-width" and "min-height" attributes on the pane splitter to "collapse-width" and "collapse-height" to better distinguish it from a CSS style.

Instead of adjusting the styling of the collapsed element directly, we leave this decision to the stylesheet. This gives more flexibility.

In particular, it allows us to set a min-height and min-width on the 3pane elements that may collapse. Whilst this would normally be redundant, if the window is resized, this ensures that the elements will not shrink below their collapse sizes.

Depends on D140498

Specifically, if we leave the collapsed state, we do not want to show a transition: it should immediately look like a non-collapsed splitter.

Depends on D140499

Attachment #9266779 - Attachment description: Bug 1755036 - Use a CSS class to indicate an element collapsed by a splitter. r=aleca,darktrojan → Bug 1755036 - Use a CSS class to indicate an element collapsed by a splitter. r=aleca

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/5a5abb91b084
Make the pane-splitter more layout independent. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/75f43bf8f968
Use a CSS class to indicate an element collapsed by a splitter. r=aleca
https://hg.mozilla.org/comm-central/rev/ddb3c3bdf606
Only show splitter transition whilst collapsed. r=aleca

If the resize-id attribute is set, but no element is found, we will continue to try and find it.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/d403f7e8db6a
Allow a pane-splitter's resizeElement to not exist when the resize-id attribute is set. r=aleca

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
See Also: → 1752150
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: