Closed Bug 1720348 Opened 3 years ago Closed 2 years ago

Dropping pill on CC or BCC buttons causes header area to have the wrong height (with reduced window height): Improve addressing area auto-height algorithm, fix splitter behaviour

Categories

(Thunderbird :: Message Compose Window, defect)

Thunderbird 92
defect

Tracking

(thunderbird_esr91 wontfix)

RESOLVED FIXED
100 Branch
Tracking Status
thunderbird_esr91 --- wontfix

People

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

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [STR and screencasts: comment 11 - 13])

Attachments

(7 files)

Steps to reproduce

  1. Open a new composition.
  2. In the "To" field, create a pill ex@org.
  3. Drag the pill on top of the "CC" or "BCC" buttons.

Result

The header area becomes slightly taller, but not tall enough such that it is now scrollable. Resizing the window does not fix this. If you delete the field, the header area remains the same height, with a larger gap between the "To" and the "Subject" fields.

Expect

The header area to grow to accommodate the new field, the same as clicking the "CC" or "BCC" buttons to add the fields.

It seems the fields added through the "mail.compose.other.header" preference can also do this:

  1. Set the "mail.compose.other.header" preference to "Other".
  2. Open a new composition.
  3. Add the "Other" header row through the ">>" menu.
  4. Create a pill in the "To" row.
See Also: → 1733409
See Also: 1733409

This is the method that takes care of calculating the header height and adding/removing the .overflow class.
https://searchfox.org/comm-central/rev/f96a8d19993aa49a1e482430c634255377b788a2/mail/components/compose/content/addressingWidgetOverlay.js#1294
It's indeed a bit wonky and it should be rewritten to better use the actual screen space.

Maybe this is also a good starting point for thinking at the new structure of the compose header, with CSS grid and a cleaner markup.

A worthwhile observation

What I've seen is that when starting new message with auto-bcc in small window (low window height), bcc row is partly covered by subject. However, I figured that this was intentional so as to keep the msg body at a certain minimum percentage of the available height compared to the addressing area (so that addressing area cannot occupy more than e.g. 40% of the window height. I think that's a reasonable behaviour (message body shouldn't shrink to less than 50% height due to addressing area), so I desisted from filing a bug (!).

Assignee: nobody → henry
Status: NEW → ASSIGNED
See Also: → 1738575

This does not reproduce on 91.0 and some of the following versions, so it's a regression of 91.3 or a bit earlier. That should help to fix it. I have some UX comments.

  • I don't think we can remove the splitter/grippy at this point unless we'd redesign the entire area to show some recipients and an intelligent overflow summary of "and 123 more Bcc recipients..." when collapsed or dragged small, which is a whole lot of work (and I'm not even sure if that would be good for everyone; I myself probably prefer to keep seeing the moderate numbers of people I write to even while I write, ymmv).

So assuming the splitter is still needed, here are some UX expectations which fix current flaws:

  • Never allow a whitespace gap between subject and recipients to appear: drag down must be limited to total height or max height of the area, whichever is smaller. This applies to dragging (down) the splitter and deleting pills.
  • Ensure that a focused address-input is always scrolled into view. This currently fails pretty often after pasting a number of recipients/pills when the header area is on overflow.
  • I guess we could try expanding the area to default height whenever a new row gets added, to ensure a better overview of existing recipients before adding more (ux-error-prevention) - this covers the scenario of bug 1738575.

In the long run, we could consider auto-collapsing the addressing area when it's not focused, and then show some recipients and an intelligent overflow summary of "and 123 more Bcc recipients...". Then auto-expand the addressing area when it gets focus. Depending on how much or little we are willing to expand by default, we might still want the splitter/grippy especially for enlargement.

Keywords: regression
Summary: Dropping pill on CC or BCC buttons causes header area to have the wrong height → Dropping pill on CC or BCC buttons causes header area to have the wrong height: Improve addressing area auto-height algorithm

(In reply to Thomas D. (:thomas8) from comment #5)

I have some UX comments.

Alex, what do you think? (see comment 5)

Flags: needinfo?(alecamar)

Sorry alecamar, this question is really to :aleca from Thunderbird team :-)

(In reply to Thomas D. (:thomas8) from comment #6)

(In reply to Thomas D. (:thomas8) from comment #5)

I have some UX comments.

Alex, what do you think? (see comment 5)

Flags: needinfo?(alecamar) → needinfo?(alessandro)

(In reply to Thomas D. (:thomas8) from comment #5)

I don't think we can remove the splitter/grippy at this point unless we'd redesign the entire area to show some recipients and an intelligent overflow summary of "and 123 more Bcc recipients..." when collapsed or dragged small, which is a whole lot of work (and I'm not even sure if that would be good for everyone; I myself probably prefer to keep seeing the moderate numbers of people I write to even while I write, ymmv).

Removing the splitter is out of scope for now.
If we will ever do something like that, we will need to carefully prototype the new method a lot before trying to implement it, as it will open up a can of worms of edge cases and usability concerns.
It's definitely doable, but not easy.

Never allow a whitespace gap between subject and recipients to appear: drag down must be limited to total height or max height of the area, whichever is smaller. This applies to dragging (down) the splitter and deleting pills.

I agree.

Ensure that a focused address-input is always scrolled into view. This currently fails pretty often after pasting a number of recipients/pills when the header area is on overflow.

This sounds good, but we will need to see it in action to be sure it's not jarring.
In general, scrolling a section to the current interaction/focus point is correct, so it shouldn't be problematic.

I guess we could try expanding the area to default height whenever a new row gets added, to ensure a better overview of existing recipients before adding more (ux-error-prevention) - this covers the scenario of bug 1738575.

Sure.

In the long run, we could consider auto-collapsing the addressing area when it's not focused, and then show some recipients and an intelligent overflow summary of "and 123 more Bcc recipients...". Then auto-expand the addressing area when it gets focus. Depending on how much or little we are willing to expand by default, we might still want the splitter/grippy especially for enlargement.

Yeah, same feedback as to the first point.

Flags: needinfo?(alessandro)
Blocks: 1744033

Henry, please note bug 1744033 which reports another related flaw (similar to bug 1738575):
Addressing area collapses to single row (without scrollbar) when splitter is single-clicked (first time only).
Perhaps you could also fix that here?

Summary: Dropping pill on CC or BCC buttons causes header area to have the wrong height: Improve addressing area auto-height algorithm → Dropping pill on CC or BCC buttons causes header area to have the wrong height: Improve addressing area auto-height algorithm, fix splitter behaviour

STR in comment 0 of this bug do not reproduce for me as-is on TB 91.3.2, Win10, System Theme (and probably didn't at the time of my comment 5 either, as I didn't explicitly confirm them). I think this depends on the condition of having a composition window with reduced height, which may affect the expected solution. It's also essentially the same which I already observed in my comment 4, so it's probably "by design", but clearly the behaviour isn't good enough.

STR - Variant A + B

Variant A (to reproduce this bug)

  1. Open a new composition.
  2. Reduce the height of composition window (from maximized, you can use Win+Right-arrow (2x), Escape, then Win+Up-arrow, so TB is now a quarter of your screen). Ensure that height of message body is about the same as the top half with addressing area and all.
  3. In the "To" field, create a pill ex@org.
  4. Drag the pill on top of the "CC" or "BCC" buttons.

(For comparison: Variant B: Repeat steps 1 to 4 with a window in full screen height.)

Actual (see Henry's comment 0, well-observed)

Variant A:

  • The header area becomes minimally taller, but not tall enough such that it is now scrollable.
  • To-row is half-hidden, making the addressing area look jumbled.
  • Resizing the window does not fix this.
  • Even if you delete the CC row, the header area remains the same height, with a larger gap between the "To" and the "Subject" fields.

(Variant B: Everything looks good as it should - this bug does not reproduce.)

Expected (complemented by :thomas8)

Variant A:

  • The addressing area should grow to accommodate the new field, the same as clicking the "CC" or "BCC" buttons to add the fields.
  • We must continue to ensure that the msg body does not get pushed off-window, so we need an absolute max-height for the top section/addressing area.
  • Find intelligent ways to manage the height according to focus. Addressing area can expand much (even 80% of window height for top section) as long as it has focus, then we could just shrink the height to something acceptable when it loses focus, unless the user has manually resized.

Here's a screencast of the STR in my comment 11, Variant A and B.
In variant A (full window height), this bug is not observed.
In variant B (reduced window height), this bug is seen.

Here's another, much more ugly and more likely variant of this bug:

STR - Variant C

  1. Start a new message
  2. Reduce composition window height (body height <= 50% of window height)
  3. In To field, add recipient foo
  4. Open CC field (Ctrl/Cmd+Shift+C)
  5. In CC field, type recipient bar, then watch what happens when you press Tab

Actual

  • Addressing area suddenly shrinks vertically
  • CC field maintains focus, but disappears behind subject whilst user is still on it and busy adding more CC recipients
  • UI looks jumbled.
  • Irritated user needs to restore the UI in clumsy ways before being able to add more CC recipients.

Expected

  • Cursor focus should remain visible
  • In this particular scenario, no reason to shrink the addressing area when the CC row is already shown and its size hasn't even changed from adding pills (but when user is pasting 100 pills, things might be different)
  • Figure out something better along expected result of comment 11.
Whiteboard: [STR and screencasts: comment 11 - 13]
Summary: Dropping pill on CC or BCC buttons causes header area to have the wrong height: Improve addressing area auto-height algorithm, fix splitter behaviour → Dropping pill on CC or BCC buttons causes header area to have the wrong height (with reduced window height): Improve addressing area auto-height algorithm, fix splitter behaviour

So, is the desired height behaviour of the header area:

  • If splitter has been clicked (and dragged) at least once, use the splitter height.
  • Else, max-height: 30% (or whatever percentage)?

And only the recipient area overflows? Specifically, neither "To" nor "Subject" are part of the overflow, so they are never scrolled out of view.

It might be easiest to convert to the new pane-splitter element, at the same time as Bug 1741361.

Flags: needinfo?(alessandro)

If splitter has been clicked (and dragged) at least once, use the splitter height.
Else, max-height: 30% (or whatever percentage)?

Correct.
I would say we can consider allowing a max height of even 40% since if users are on a 720p laptop, 30% of the height of the compose window might be very narrow.

And only the recipient area overflows? Specifically, neither "To" nor "Subject" are part of the overflow, so they are never scrolled out of view.

You meant "From" and "Subject", right?
The To field is part of the recipients area and I think it should be part of the scrollable area.

It might be easiest to convert to the new pane-splitter element, at the same time as Bug 1741361.

Sounds good.
You can maybe first update the splitter, and then do the rest in a separate patch to keep things smaller and quicker to review.

Flags: needinfo?(alessandro)

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

If splitter has been clicked (and dragged) at least once, use the splitter height.
Else, max-height: 30% (or whatever percentage)?

Correct.
I would say we can consider allowing a max height of even 40% since if users are on a 720p laptop, 30% of the height of the compose window might be very narrow.

ok, thanks!

And only the recipient area overflows? Specifically, neither "To" nor "Subject" are part of the overflow, so they are never scrolled out of view.

You meant "From" and "Subject", right?

Yup. Sorry, I got them mixed up.

Depends on: 1755036

Renamed the "id"s to emphasise that this is a pane for contacts and to use camel-case.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/cfbe2da6989a
Rename or remove the "id"s used in compose contacts sidebar. r=aleca

The new ids are more specific to the compose message context.

Depends on D141382

Also moved any padding that was in #headers-box, or the removed #addresses-box and #msgheaderstoolbar-box, into #MsgHeadersToolbar and #FormatToolbar.

Depends on D141385

Blocks: 1760773

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/66290e800b5c
Rename "appcontent" and "content-frame" ids. r=aleca
https://hg.mozilla.org/comm-central/rev/a5f4459d620c
Use a grid display on MsgHeadersToolbar. r=aleca
https://hg.mozilla.org/comm-central/rev/864e87ddf020
Use html splitters in the compose window. r=aleca

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch
Regressions: 1761617

Forgot the change the indent in rev 864e87ddf020

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

D142665 need unbitrotting

(In reply to Magnus Melin [:mkmelin] from comment #25)

D142665 need unbitrotting

It has been rebased now.

Pushed by nicolai@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/9ad0831a37cd
Fix indent in messengercompose.xhtml after rev 864e87ddf020. r=freaktechnik,john.bieling

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

Attachment

General

Created:
Updated:
Size: