Move the splitter on top of the FormatToolbar

RESOLVED FIXED in Thunderbird 7.0

Status

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 7.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

7 years ago
In Bug 654754 Comment 13 bwinton sayd:

> And I would still prefer to see the splitter on top of the FormatToolbar,
> but we can leave that for another bug, I guess…

This makes sense because the splitter changes the height of the addressing widget and the FormatToolbar is more associated to the text content.
(Assignee)

Comment 1

7 years ago
Created attachment 534293 [details] [diff] [review]
Move the splitter

This patch moves the splitter above the FormatToolbar.

Under Win7 I gave the tool bar a 1px top border because the splitter is invisible (mouse pointer still changes). If needed I can make it 2px high.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #534293 - Flags: ui-review?(nisses.mail)

Comment 2

7 years ago
> This makes sense because the splitter changes the height of the addressing
> widget and the FormatToolbar is more associated to the text content.

On the other hand, one could argue that the splitter determines the ratio between the header pane and the message pane in the composition window, which is allocated to the address rows. This was probably the thinking when initially implementing it in the current way.

On the other other hand, the formatting toolbar can be considered part of the message, thus moving the splitter on top of it makes equally sense.  :-)

> I gave the tool bar a 1px top border because the splitter is invisible

Wasn't the active height made 3px somewhere for better accessibility or is this not touched by your patch? Grabbing 1px with the mouse may be tricky for some.
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)
> Wasn't the active height made 3px somewhere for better
> accessibility or is this not touched by your patch? Grabbing 1px
> with the mouse may be tricky for some.

My description was unclear. The splitter is still 4px heigh but isn't visible with a different color. Actually in TB 3.4 under Win7 you see only the border between the header box and the message. The splitter has still his 4px space. The border on top of the FormatToolbar copies only this behavior.
I think this change makes sense, but I had issues getting it to work on my system (I think). Could you provide some screenshots of how it's supposed to look like?
(Assignee)

Comment 5

7 years ago
Created attachment 536314 [details]
Patch in action under Win7

Screenshot of patch in action under Win7 Aero and Classic
Comment on attachment 534293 [details] [diff] [review]
Move the splitter

Based on the screenshots (sorry, build still broken), ui-r+ from me
Attachment #534293 - Flags: ui-review?(nisses.mail) → ui-review+
(Assignee)

Updated

7 years ago
Attachment #534293 - Flags: review?(bwinton)
Comment on attachment 534293 [details] [diff] [review]
Move the splitter

Review of attachment 534293 [details] [diff] [review]:
-----------------------------------------------------------------

Other than the change I mention below (and the other instances of that change), I think I like this patch, so r=me with either that change made, or a good explanation of why you can't and why it's not so bad.  ;)

Thanks,
Blake.

::: mail/components/compose/content/messengercompose.xul
@@ +87,4 @@
>  .toolbarbutton-menubutton-button,
>  .toolbarbutton-menubutton-dropmarker,
>  .toolbarbutton-1,
> +menulist {

That seems like a far-reaching change…  Are you sure you don't want to change it to "#FormatToolbox menulist" instead?
Attachment #534293 - Flags: review?(bwinton) → review+
(Assignee)

Comment 8

7 years ago
It would not only need a "#FormatToolbar menulist", also a "#addresses-box menulist". Because messengercompose.css affects only the messengercompose window I thought, I can use such a wide rule.

If you still decide I should add these stronger rules, I'll them.
(In reply to comment #8)
> It would not only need a "#FormatToolbar menulist", also a "#addresses-box
> menulist". Because messengercompose.css affects only the messengercompose
> window I thought, I can use such a wide rule.
> 
> If you still decide I should add these stronger rules, I'll them.

No, that's a good explanation of why you don't want to, and why it's not so bad.  :)  The r=me stands.

Later,
Blake.
(Assignee)

Updated

7 years ago
Keywords: checkin-needed

Updated

7 years ago
Blocks: 645294
Checked in: http://hg.mozilla.org/comm-central/rev/585da56d82a4
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 7.0
Duplicate of this bug: 671254
You need to log in before you can comment on or make changes to this bug.