OS X - Update Compose Window UI

RESOLVED FIXED in Thunderbird 29.0

Status

enhancement
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jsbruner, Assigned: jsbruner)

Tracking

(Blocks 1 bug)

unspecified
Thunderbird 29.0
All
macOS
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ux-feature-wanted-31], URL)

Attachments

(7 attachments, 15 obsolete attachments)

262.31 KB, image/png
Details
234.81 KB, image/png
Details
239.70 KB, image/png
Details
63.84 KB, image/png
Details
181.62 KB, image/png
Details
96.29 KB, image/png
jsbruner
: feedback-
Details
10.70 KB, patch
jsbruner
: review+
jsbruner
: ui-review+
Details | Diff | Splinter Review
This bug: Update the Compose Window's current UI to one of these mockups:

https://pancake.io/42b819/TBComposerMergeNew5.html

https://pancake.io/42b819/TBComposerMergeNew4.html

https://pancake.io/42b819/TBComposerInsetMerge.html

The differences are really only related to the shadowing, so that can be changed on the fly.

Not seen in the mockup, but the <select> elements will have custom styling as well to look more appealing. Unfortunately, gecko does not allow custom styling yet, but TB will work successfully.

This is step A.

Step B is doing the same to Linux.

Step C is doing the same to Windows.

The goal is to land all three of these in version 25 or perhaps earlier. For consistency though, we should try to land all three in the same version.

Comment 1

6 years ago
(In reply to Josiah Bruner [:JosiahOne] from comment #0)
> This bug: Update the Compose Window's current UI to one of these mockups:
> 
> https://pancake.io/42b819/TBComposerMergeNew5.html
> 
> https://pancake.io/42b819/TBComposerMergeNew4.html
> 
> https://pancake.io/42b819/TBComposerInsetMerge.html
> 
> The differences are really only related to the shadowing, so that can be
> changed on the fly.

I can not open this links, any redirection error :(
But if you say "shadow", is this than similar to Bug 554113?
(In reply to Nomis101 from comment #1)
> (In reply to Josiah Bruner [:JosiahOne] from comment #0)
> > This bug: Update the Compose Window's current UI to one of these mockups:
> > 
> > https://pancake.io/42b819/TBComposerMergeNew5.html
> > 
> > https://pancake.io/42b819/TBComposerMergeNew4.html
> > 
> > https://pancake.io/42b819/TBComposerInsetMerge.html
> > 
> > The differences are really only related to the shadowing, so that can be
> > changed on the fly.
> 
> I can not open this links, any redirection error :(
> But if you say "shadow", is this than similar to Bug 554113?

Not sure why the redirection error is affecting you, everyone else was able to view it. I can right now.

Anyway, no, this does not simply add a shadow, I meant the differences in those mockups are only related to the shadows.

I'll go upload some images of the mockups...
Posted image Mockup #5 β€”
Posted image Mockup #4 β€”
Posted image Mockup #2 β€”

Comment 6

6 years ago
So, you are suggesting to get rid of the textbox/listbox-style address area and to enter recipients on those lines? Not having a Mac myself (as a disclaimer), I'm wondering how that fits with the expected UI guidelines on that (or any other) platform. It's a rather unusual way to style an input area?
(In reply to rsx11m from comment #6)
> So, you are suggesting to get rid of the textbox/listbox-style address area
> and to enter recipients on those lines? Not having a Mac myself (as a
> disclaimer), I'm wondering how that fits with the expected UI guidelines on
> that (or any other) platform. It's a rather unusual way to style an input
> area?

Fair point. I would definitely agree with you if the custom styling caused the expected action to become questionable, however the styling quite clearly indicates an input field. 

And how input fields appear varies quite a bit actually.

Take Air Mail, it actually doesn't have any text field at all really, instead, each field view is broken up by lines. (I'll attach a screenshot in a second).

However, Blake, do you have an issue with the lines instead of entire fields for the UI?

Comment 9

6 years ago
(In reply to Josiah Bruner [:JosiahOne] from comment #7)
> Take Air Mail, it actually doesn't have any text field at all really,
> instead, each field view is broken up by lines.

This looks a bit different, though. The lines go all the way from left to right in that application, thus separating the fields, and each field has a top and bottom boundaries. In contrast, the mockups have just the line at the bottom, the first entry doesn't have an upper boundary, and the Subject line isn't clearly separated from the address block any more. At least initially, I find that confusing.

Comment 10

6 years ago
I'm not that averse of this lines. It's a more skeuomorphic approach. It imitates a postcard where you also find this lines to write the addressee on. But I would make a better differentiation between the addressee and the subject.

Comment 11

6 years ago
I'm not saying that I'm opposed to it, I was only voicing concerns that it may be confusing as it's an interesting but unexpected approach. ;-) Yes, the Subject line should be more clearly separated, and it should also help to have some hover/focus effects on those lines to identify the box where it is relevant.

See also bug 425451 regarding the number of initially offered address lines.
(In reply to rsx11m from comment #11)
> I'm not saying that I'm opposed to it, I was only voicing concerns that it
> may be confusing as it's an interesting but unexpected approach. ;-) Yes,
> the Subject line should be more clearly separated, and it should also help
> to have some hover/focus effects on those lines to identify the box where it
> is relevant.
> 
> See also bug 425451 regarding the number of initially offered address lines.

Yes, I actually do agree that the subject line could be differentiated more, and I would like the idea of fading in each field on hover actions. That would make the purpose very clear for those who aren't completely sure of their purpose.

> interesting but unexpected approach...

Hehe. Yeah, that how I roll. ;) But seriously, I appreciate your feedback, definitely valid points. We can make some changes after I have a first-patch ready. (Either tomorrow or Thursday)
Posted patch WIP (obsolete) β€” β€” Splinter Review
Current patch. Still needs a lot of work, but is definitely getting there...
Posted patch WIP 2 (obsolete) β€” β€” Splinter Review
Newest patch. Almost complete, but missing some finishing touches such as:

"Adding a dropdown arrow, although I created the image and added it here."

"Fading in textfields onHover"
Attachment #744786 - Attachment is obsolete: true
Depends on: 865670
Status: NEW → ASSIGNED
Posted patch WIP (obsolete) β€” β€” Splinter Review
Getting pretty close to finishing up this one. Just check-pointing some progress here.

ToDo:

* #msgSubject border
* Address book panel separator
* Code cleanup.
Attachment #747614 - Attachment is obsolete: true
Posted patch WIP (obsolete) β€” β€” Splinter Review
Almost done. Only code cleanup next. Asking Richard for any final feedback on the UI.
Attachment #763280 - Attachment is obsolete: true
Attachment #790941 - Flags: feedback?(richard.marti)
Target Milestone: Thunderbird 25.0 → Thunderbird 26.0
Comment on attachment 790941 [details] [diff] [review]
WIP

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

First, I like how it looks now.

Now why I minused it:
- In patch it has a .rej file
- If arrowpicker.png and osx-noise.png are only used in compose then put them in
  messengercompose folder
- The textboxes should not only on hover show the normal textbox appearance. They
  should also have this when they are focused to show the function.
- The splitter looks a little bit blurry and for me outplaced. What if you style
  it like the shadow in Mockup #5?

::: mail/components/compose/content/messengercompose.xul
@@ +970,2 @@
>  
> +      <splitter id="compose-toolbar-sizer" onmousedown="awSizerListen()"/>

The splitter should be indented like the toolbox.

::: mail/themes/osx/mail/compose/messengercompose.css
@@ +417,5 @@
>  
>  /* ::::: special toolbar colors ::::: */
>  
> +#thatTopBoxThere {
> +	box-shadow: 0px 0px 0px 0px rgba(0, 0, 0, 0.3), 0px 2px 4px rgba(0, 0, 0, 0.6) inset;

Please use spaces. Where are a lot of tabs used in this file now.

@@ +435,5 @@
> +	box-shadow: none;
> +}
> +
> +#appcontent {
> +  margin-right: 3px;

Please use -moz-margin-end to be rtl compatible.

@@ +439,5 @@
> +  margin-right: 3px;
> +}
> +
> +#attachments-box {
> +	padding-right: 3px;

And here -moz-padding-end.
I'm not marking every existence of this type. Please change all which can affect rtl.

@@ +682,5 @@
> +  background-image: url("chrome://messenger/skin/osx-noise.png");
> +  background-color: rgb(242, 242, 242);
> +  border-bottom-style: solid;
> +  border-bottom-width: 1px;
> +  border-bottom-color: rgb(220, 220, 220);

border-bottom: 1px solid #DCDCDC; is the same as the three lines. #DCDCDC is already used for other borders, so it is easier to use this instead of rgb(220, 220, 220) to find the same colors in this file.
Attachment #790941 - Flags: feedback?(richard.marti) → feedback-
Posted patch Patch. (obsolete) β€” β€” Splinter Review
Done. \o/ (For now at least)
Attachment #790941 - Attachment is obsolete: true
Attachment #791434 - Flags: review?(richard.marti)
Comment on attachment 791434 [details] [diff] [review]
Patch.

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

UI wise it looks great = ui-r+
But the code has to much which needs a fix = r-

::: mail/components/compose/content/messengercompose.xul
@@ +797,5 @@
>  
>    <toolbarset id="customToolbars" context="toolbar-context-menu"/>
>    </toolbox>
>  
> +  <hbox flex="1" id="thatTopBoxThere">

Could you give this a better name which describes the function better? Maybe composeContent-box or something other.

@@ +832,5 @@
>            <listbox id="addressingWidget" flex="1" seltype="multiple" rows="1"
>                     onkeydown="awKeyDown(event, this)"
>                     onclick="awClickEmptySpace(event.originalTarget, true)"
>                     disableonsend="true">
> +            <listcols id="listcolumns">

Is the ID needed?

@@ +899,5 @@
>        </hbox>
>      </toolbar>
> +
> +      <!-- These toolbar items get filled out from the editorOverlay -->
> +      <toolbox id="FormatToolbox" mode="icons">

The whole block is indented 2px to much.

::: mail/themes/osx/mail/compose/messengercompose.css
@@ +8,5 @@
>  
>  @import url("chrome://messenger/skin/");
>  
>  @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
> +@namespace html url("http://www.w3.org/1999/xhtml");

Is this needed?

@@ +419,5 @@
>  
> +#thatTopBoxThere {
> +	box-shadow: 0px 2px 4px rgba(0, 0, 0, 0.6) inset;
> +	background-color: rgb(242, 242, 242);
> +	

Almost all lines in this block have tabs instead of spaces.

@@ +431,5 @@
> +	margin-left: -3px;
> +}
> +
> +#thatTopBoxThere:-moz-window-inactive {
> +	box-shadow: 0px 1px 2px rgba(0, 0, 0, 0.2) inset;

Use spaces.

@@ +449,5 @@
> +  -moz-margin-end: 3px;
> +}
> +
> +#attachments-box {
> +	-moz-padding-end: 3px;

Use spaces.

@@ +459,1 @@
>  }

#headers-box should be no more needed.

@@ +464,2 @@
>    border-bottom: 0px solid;
> +  background-color: transparent;

background-color shouldn't be needed.

@@ +517,3 @@
>    -moz-margin-end: 1px;
> +  background-color: inherit;
> +  -moz-appearance: none !important;

The -moz-appearance isn't needed.

@@ +517,4 @@
>    -moz-margin-end: 1px;
> +  background-color: inherit;
> +  -moz-appearance: none !important;
> +  border-shadow: none !important;

Unknown property.

@@ +522,5 @@
> +  border-left: 1px solid transparent;
> +  border-right: 1px solid transparent;
> +  border-bottom: 1px solid #898989;
> +  border-bottom-left-radius: 0px !important;
> +  border-top-right-radius: 0px !important;

The !important isn't needed for both lines.

@@ +530,5 @@
> +  -moz-border-bottom-colors: none;
> +  padding: 2px;
> +  -moz-padding-start: 5px !important;
> +  transition: border .2s, background-color .2s;
> +  -moz-transition: border .2s, background-color .2s;

-moz-transition is not needed when transition is set. -moz-transition is for Gecko <16.

@@ +537,5 @@
> +#msgSubject:hover, #msgSubject[focused="true"] {
> + 	background-color: white;
> + 	background-image: none;
> +  border-top: 1px solid #DCDCDC;
> +	border-left: 1px solid #DCDCDC;

Use spaces.

@@ +614,5 @@
>    background-color: inherit !important;
>    color: inherit !important;
>  }
>  
> +.textbox-addressingWidget, .dummy-row-cell:not(first-child) {

One selector per line please.
And .dummy-row-cell:not(first-child) should be .dummy-row-cell:not(:first-child).

@@ +619,5 @@
> +  border-top: 1px solid transparent !important;
> +  border-left: 1px solid transparent !important;
> +  border-right: 1px solid transparent !important;
> +  border-bottom: 1px solid #DCDCDC !important;
> +  -moz-border-colors: none !important;

Unknown property. Use
-moz-border-top-colors: none;
-moz-border-left-colors: none;
-moz-border-right-colors: none;
-moz-border-bottom-colors: none;

@@ +623,5 @@
> +  -moz-border-colors: none !important;
> +  padding: 2px !important;
> +
> +  transition: border .2s, background-color .2s;
> +  -moz-transition: border .2s, background-color .2s;

-moz-transition is not needed when transition is set.

@@ +626,5 @@
> +  transition: border .2s, background-color .2s;
> +  -moz-transition: border .2s, background-color .2s;
> +}
> +
> +.textbox-addressingWidget[focused="true"], .textbox-addressingWidget:hover {

One selector per line please.

@@ +630,5 @@
> +.textbox-addressingWidget[focused="true"], .textbox-addressingWidget:hover {
> + 	background-color: white;
> + 	background-image: none;
> +  border-top: 1px solid #DCDCDC !important;
> +	border-left: 1px solid #DCDCDC !important;

Use spaces.

@@ +642,1 @@
>  }

This selectors should be no more needed when .dummy-row-cell:not(first-child) is fixed.

@@ +673,5 @@
>  
>  /* ::::: format toolbar ::::: */
>  
>  #FormatToolbar {
> +  -moz-appearance: none;

Should be not needed. Is already defined by line 47.
Attachment #791434 - Flags: ui-review+
Attachment #791434 - Flags: review?(richard.marti)
Attachment #791434 - Flags: review-
Posted patch Patch. (obsolete) β€” β€” Splinter Review
Made changes with a few exceptions which I will list in the next comment. (Patch comments don't allow an easy way to quote)
Attachment #791434 - Attachment is obsolete: true
Attachment #791678 - Flags: ui-review+
Attachment #791678 - Flags: review?(richard.marti)
Posted patch Patch. (obsolete) β€” β€” Splinter Review
Missed something. Anyway, here are the changes:


(In reply to Richard Marti [:Paenglab] from comment #19)
> Comment on attachment 791434 [details] [diff] [review]
> Patch.
> 
> Review of attachment 791434 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> UI wise it looks great = ui-r+
> But the code has to much which needs a fix = r-
> 
> ::: mail/components/compose/content/messengercompose.xul
> @@ +797,5 @@
> >  
> >    <toolbarset id="customToolbars" context="toolbar-context-menu"/>
> >    </toolbox>
> >  
> > +  <hbox flex="1" id="thatTopBoxThere">
> 
> Could you give this a better name which describes the function better? Maybe
> composeContent-box or something other.

Done.

> 
> @@ +832,5 @@
> >            <listbox id="addressingWidget" flex="1" seltype="multiple" rows="1"
> >                     onkeydown="awKeyDown(event, this)"
> >                     onclick="awClickEmptySpace(event.originalTarget, true)"
> >                     disableonsend="true">
> > +            <listcols id="listcolumns">
> 
> Is the ID needed?

Nope, fixed.

> 
> @@ +899,5 @@
> >        </hbox>
> >      </toolbar>
> > +
> > +      <!-- These toolbar items get filled out from the editorOverlay -->
> > +      <toolbox id="FormatToolbox" mode="icons">
> 
> The whole block is indented 2px to much.

Fixed.

> 
> ::: mail/themes/osx/mail/compose/messengercompose.css
> @@ +8,5 @@
> >  
> >  @import url("chrome://messenger/skin/");
> >  
> >  @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
> > +@namespace html url("http://www.w3.org/1999/xhtml");
> 
> Is this needed?

Nope, fixed.

> 
> @@ +419,5 @@
> >  
> > +#thatTopBoxThere {
> > +	box-shadow: 0px 2px 4px rgba(0, 0, 0, 0.6) inset;
> > +	background-color: rgb(242, 242, 242);
> > +	
> 
> Almost all lines in this block have tabs instead of spaces.

Fixed.

> 
> @@ +431,5 @@
> > +	margin-left: -3px;
> > +}
> > +
> > +#thatTopBoxThere:-moz-window-inactive {
> > +	box-shadow: 0px 1px 2px rgba(0, 0, 0, 0.2) inset;
> 
> Use spaces.

Done.

> 
> @@ +449,5 @@
> > +  -moz-margin-end: 3px;
> > +}
> > +
> > +#attachments-box {
> > +	-moz-padding-end: 3px;
> 
> Use spaces.

Done.

> 
> @@ +459,1 @@
> >  }
> 
> #headers-box should be no more needed.

Yep, done.

> 
> @@ +464,2 @@
> >    border-bottom: 0px solid;
> > +  background-color: transparent;
> 
> background-color shouldn't be needed.

Done.

> 
> @@ +517,3 @@
> >    -moz-margin-end: 1px;
> > +  background-color: inherit;
> > +  -moz-appearance: none !important;
> 
> The -moz-appearance isn't needed.

No, it is needed. Change not made.

> 
> @@ +517,4 @@
> >    -moz-margin-end: 1px;
> > +  background-color: inherit;
> > +  -moz-appearance: none !important;
> > +  border-shadow: none !important;
> 
> Unknown property.

Fixed.

> 
> @@ +522,5 @@
> > +  border-left: 1px solid transparent;
> > +  border-right: 1px solid transparent;
> > +  border-bottom: 1px solid #898989;
> > +  border-bottom-left-radius: 0px !important;
> > +  border-top-right-radius: 0px !important;
> 
> The !important isn't needed for both lines.

Done.

> 
> @@ +530,5 @@
> > +  -moz-border-bottom-colors: none;
> > +  padding: 2px;
> > +  -moz-padding-start: 5px !important;
> > +  transition: border .2s, background-color .2s;
> > +  -moz-transition: border .2s, background-color .2s;
> 
> -moz-transition is not needed when transition is set. -moz-transition is for
> Gecko <16.

Done.

> 
> @@ +537,5 @@
> > +#msgSubject:hover, #msgSubject[focused="true"] {
> > + 	background-color: white;
> > + 	background-image: none;
> > +  border-top: 1px solid #DCDCDC;
> > +	border-left: 1px solid #DCDCDC;
> 
> Use spaces.

Done.

> 
> @@ +614,5 @@
> >    background-color: inherit !important;
> >    color: inherit !important;
> >  }
> >  
> > +.textbox-addressingWidget, .dummy-row-cell:not(first-child) {
> 
> One selector per line please.
> And .dummy-row-cell:not(first-child) should be
> .dummy-row-cell:not(:first-child).

Moved to a new line, however .dummy-row-cell:not(:first-child) does not work at all. I was correct before.

> 
> @@ +619,5 @@
> > +  border-top: 1px solid transparent !important;
> > +  border-left: 1px solid transparent !important;
> > +  border-right: 1px solid transparent !important;
> > +  border-bottom: 1px solid #DCDCDC !important;
> > +  -moz-border-colors: none !important;
> 
> Unknown property. Use
> -moz-border-top-colors: none;
> -moz-border-left-colors: none;
> -moz-border-right-colors: none;
> -moz-border-bottom-colors: none;

I just removed it entirely.

> 
> @@ +623,5 @@
> > +  -moz-border-colors: none !important;
> > +  padding: 2px !important;
> > +
> > +  transition: border .2s, background-color .2s;
> > +  -moz-transition: border .2s, background-color .2s;
> 
> -moz-transition is not needed when transition is set.

Done.

> 
> @@ +626,5 @@
> > +  transition: border .2s, background-color .2s;
> > +  -moz-transition: border .2s, background-color .2s;
> > +}
> > +
> > +.textbox-addressingWidget[focused="true"], .textbox-addressingWidget:hover {
> 
> One selector per line please.

Done.

> 
> @@ +630,5 @@
> > +.textbox-addressingWidget[focused="true"], .textbox-addressingWidget:hover {
> > + 	background-color: white;
> > + 	background-image: none;
> > +  border-top: 1px solid #DCDCDC !important;
> > +	border-left: 1px solid #DCDCDC !important;
> 
> Use spaces.

Done.

> 
> @@ +642,1 @@
> >  }
> 
> This selectors should be no more needed when
> .dummy-row-cell:not(first-child) is fixed.

It is needed. Change not made.

> 
> @@ +673,5 @@
> >  
> >  /* ::::: format toolbar ::::: */
> >  
> >  #FormatToolbar {
> > +  -moz-appearance: none;
> 
> Should be not needed. Is already defined by line 47.

Yup, fixed.
Attachment #791678 - Attachment is obsolete: true
Attachment #791678 - Flags: review?(richard.marti)
Attachment #791681 - Flags: ui-review+
Attachment #791681 - Flags: review?(richard.marti)
Comment on attachment 791681 [details] [diff] [review]
Patch.

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

Still some issues but I'm sure the next version is the last one :)

(In reply to Josiah Bruner [:JosiahOne] from comment #21)
> Created attachment 791681 [details] [diff] [review]
> Patch.
> > > +.textbox-addressingWidget, .dummy-row-cell:not(first-child) {
> > 
> > One selector per line please.
> > And .dummy-row-cell:not(first-child) should be
> > .dummy-row-cell:not(:first-child).
> 
> Moved to a new line, however .dummy-row-cell:not(:first-child) does not work
> at all. I was correct before.

No, :not(first-child) doesn't check for the *pseudo* element first-child but for a not existing element first-child. : is the sign for pseudo element. You can check with DOMi this selector is also applied on the first child.

> > @@ +642,1 @@
> > >  }
> > 
> > This selectors should be no more needed when
> > .dummy-row-cell:not(first-child) is fixed.
> 
> It is needed. Change not made.

Splinter showed the wrong line. What I mean wass:

+.addressingWidgetCell:first-child,
+.dummy-row-cell:first-child {
+  border: none !important;
 }

which isn't needed with the :not(:first-child) fix.
I tried this it and it works, sure.

::: mail/components/compose/content/messengercompose.xul
@@ +901,2 @@
>  
> +      <!-- These toolbar items get filled out from the editorOverlay -->

Two spaces to much.

@@ +903,3 @@
>      <toolbox id="FormatToolbox" mode="icons">
>        <toolbar class="chromeclass-toolbar" id="FormatToolbar" persist="collapsed"
> +        customizable="true" nowindowdrag="true">

Now every line with attributes is wrong indented in this toolbar.

@@ +974,2 @@
>  
>      <editor type="content-primary" id="content-frame" src="about:blank" name="browser.message.body" flex="1"

Please remove the line between vbox and editor.

::: mail/themes/osx/mail/compose/messengercompose.css
@@ +527,5 @@
> +
> +#msgSubject:hover, 
> +#msgSubject[focused="true"] {
> + 	background-color: white;
> + 	background-image: none;

Where are still tabs in this two lines.

@@ +625,5 @@
>  
> +.textbox-addressingWidget[focused="true"],
> +.textbox-addressingWidget:hover {
> + 	background-color: white;
> + 	background-image: none;

Again, where are still tabs in this two lines.
Attachment #791681 - Flags: review?(richard.marti) → review-
Posted patch Patch. (obsolete) β€” β€” Splinter Review
Changes made, thanks Richard!

Sorry about the tab problem, Xcode must be doing something wrong since it clearly is showing up as spaces there.
Attachment #791681 - Attachment is obsolete: true
Attachment #791739 - Flags: ui-review+
Attachment #791739 - Flags: review?(richard.marti)
Posted patch Patch. (obsolete) β€” β€” Splinter Review
Removed trailing whitespace...
Attachment #791739 - Attachment is obsolete: true
Attachment #791739 - Flags: review?(richard.marti)
Attachment #791740 - Flags: ui-review+
Attachment #791740 - Flags: review?(richard.marti)
Comment on attachment 791740 [details] [diff] [review]
Patch.

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

Great, r+
Attachment #791740 - Flags: review?(richard.marti) → review+
Posted patch Final Patch. (obsolete) β€” β€” Splinter Review
Final version with some changes to the Account menulist. Made it look similar to the other fields.

Carrying ui-review and review flags, though Richard: If you want to quickly review the changes go ahead and do so.
Attachment #791740 - Attachment is obsolete: true
Attachment #792985 - Flags: ui-review+
Attachment #792985 - Flags: review+
Posted patch Final patch. (obsolete) β€” β€” Splinter Review
Updating commit username.
Attachment #792985 - Attachment is obsolete: true
Attachment #800792 - Flags: ui-review+
Attachment #800792 - Flags: review+

Comment 29

6 years ago
Is there something missing in the patch?? It's already reviewed but not set for checkin-needed?
(In reply to Nomis101 from comment #29)
> Is there something missing in the patch?? It's already reviewed but not set
> for checkin-needed?

I have to land this one the same time as the Linux and Windows implementations. Those two aren't yet done, though they should be within a month.

Comment 31

6 years ago
Hey Josiah, interesting new smooth layout, with all those edges and boxes ironed away... :)

(In reply to rsx11m from comment #9)
> (In reply to Josiah Bruner [:JosiahOne] from comment #7)
> > Take Air Mail, it actually doesn't have any text field at all really,
> > instead, each field view is broken up by lines.
> 
> This looks a bit different, though. The lines go all the way from left to
> right in that application, thus separating the fields, and each field has a
> top and bottom boundaries. In contrast, the mockups have just the line at
> the bottom, the first entry doesn't have an upper boundary, and the Subject
> line isn't clearly separated from the address block any more. At least
> initially, I find that confusing.

+1. The airmail design is more functional with neat horizontal boundaries for each input. It would be interesting to try these long horizontal lines on Josia's attachment 793022 [details], but I'm not sure we'll get there without bug 440377 (which is where we differ from airmail due to TB's overcome one-line-per-recipient input).

Like rsx11m's comment 11 and Richard's comment 17, I'm also interested in the ux-discovery aspect of this. Could you attach a screenshots of the following?
- hover effect (e.g. hovering subject)
- focus effect (e.g. cursor after "foo bar" in subject)
- compose window with 1 or more file attachments (after adding first attachment)
- compose window with empty attachment pane (after deleting last attachment)
I'll run a try build later today for people to test. But remember though that the UI-review was +'d, which indicates that the UI is discoverable and designed well. Therefore, unless there is a serious flaw, additional input may not be added. Especially when the other platforms' implementation is not yet complete.

Updated

6 years ago
Attachment #793022 - Attachment description: Screenshot of new look → Screenshot 1: The new look

Updated

6 years ago
Attachment #743848 - Attachment description: Air Mail Compose Window → Screenshot A: Air Mail Compose Window

Comment 33

6 years ago
As part of our cooperative volunteer effort to work for the common goal, a better Thunderbird... here's a tentative mockup of compose header with just a slight variation of Josiah's great design work seen in Screenshot 1, attachment 793022 [details].

- long lines for neater functional divisions between From header, Address widget, Subject, and Formatting Toolbar
- inspired by Airmail, see Screenshot A, attachment 743848 [details]

Rsx11m's comment 9 explains in detail why this might be helpful for a more structured appearance, allowing easy visual identification and distinguation of different UI parts. Looks more "peaceful" to me. Neater subject line, avoiding outstanding dark-grey underlining of subject, which I find irritating given the overall smooth look provided by Josiah.

(This mockup is tentative as it still needs some tweaking of vertical distances between the long dividing lines; can we recover some vertical screen real estate while we're here? E.g. As splitter is now arguably in more intuitive place below formatting toolbar and blending in with same, we could try reducing its height by some px...)
Comment on attachment 805441 [details]
Screenshot 2: Compose header pane with long lines for neater functional divisions as in airmail (see Screenshot A)

Assuming you wanted UI-feedback on this so adding the flag. :)

While in theory this is nice, our compose window doesn't really suit such a design.

1. We don't want so many lines as in your mockup as it causes confusion and distraction. You have some lines for separation and some for input, which will definitely confuse the user as to the function of the field. This amount of lines also comes across a little cumbersome and heavy, which we don't want and is the reason this bug exists.

2. The subject field was specifically created to be darker to separate it from the To: fields. And while it may not seem useful to many users, our business users will greatly appreciate it. I'll explain...

The reason for this is because we have designed our window to allow sending to a large amount of people easy, quick to navigate through, and customizable. Unfortunately, this means a *lot* of lines. Without separating the subject field enough, it is usually hard to tell if the subject line is for the subject or for adding more people to the email.

So although this works for some email clients (like Airmail), in our case it doesn't so much. Remember, we're our own product and needn't follow other email client's direction. Airmail by design is focused more on the UI and less on ease-of-use. We in the past have strove more towards the latter.

Thanks,
Josiah
Attachment #805441 - Flags: ui-review-

Comment 35

6 years ago
Josiah, are you a formal ui-reviewer for the Thunderbird component?
If not, please use feedback- at most for expressing your disagreement.
Comment on attachment 805441 [details]
Screenshot 2: Compose header pane with long lines for neater functional divisions as in airmail (see Screenshot A)

Not technically, though in this case the flag was also based on Richard Marti's opinion, which matched mine. So that's why I ui-review-'d it. I suppose he could do it, though unnecessary. But I'll switch to feedback-.

Regardless, like I said, the current patch has been ui-review+'d and review+'d so the design isn't really up for debate anymore. Unless of course there is a serious bug in the patch. :)
Attachment #805441 - Flags: ui-review- → feedback-

Comment 37

6 years ago
Thanks. The proper procedure for Thomas would be to file his proposal in a new bug once this has been checked in to modify the current design, unless one of the reviewers picks it up here and wants the change. Receiving a ui-r+ doesn't imply that the design cannot subsequently be changed... ;-)
(In reply to rsx11m from comment #37)
> Thanks. The proper procedure for Thomas would be to file his proposal in a
> new bug once this has been checked in to modify the current design, unless
> one of the reviewers picks it up here and wants the change. Receiving a
> ui-r+ doesn't imply that the design cannot subsequently be changed... ;-)

Oh for sure. I'm just saying in *this* bug we probably won't change the design. :)

Try build started:

Results will be displayed on TBPL as they come in:
https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=01308009d1db

Once completed, builds and logs will be available at:
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/josiah@programmer.net-01308009d1db
Posted patch Final Patch. (obsolete) β€” β€” Splinter Review
Updated for trunk. Carrying review+ flags.
Attachment #800792 - Attachment is obsolete: true
Attachment #814874 - Flags: ui-review+
Attachment #814874 - Flags: review+
Posted patch Final patch. (obsolete) β€” β€” Splinter Review
Updated for trunk.
Attachment #814874 - Attachment is obsolete: true
Attachment #831556 - Flags: ui-review+
Attachment #831556 - Flags: review+
Attachment #831556 - Flags: feedback+
Posted patch Final Patch. (obsolete) β€” β€” Splinter Review
Accidentally left a remove rej file in that last patch. Removed here.
Attachment #831556 - Attachment is obsolete: true
Attachment #831563 - Flags: ui-review+
Attachment #831563 - Flags: review+
Attachment #831563 - Flags: feedback+

Comment 42

6 years ago
In which TB version will we see this? Currently its saying the target is TB 26...
(In reply to Nomis101 from comment #42)
> In which TB version will we see this? Currently its saying the target is TB
> 26...

Sorry, forgot to update that. Currently the plan is to have the whole feature arrive in Thunderbird 31. But for testing purposes, ideally this should land is TB 30.
Target Milestone: Thunderbird 26.0 → Thunderbird 30.0
Whiteboard: [ux-feature-wanted-31]
Posted patch Final Patch. (obsolete) β€” β€” Splinter Review
Updated for trunk.
Attachment #831563 - Attachment is obsolete: true
Attachment #8347286 - Flags: ui-review+
Attachment #8347286 - Flags: review+
Attachment #8347286 - Flags: feedback+
Posted patch Final Patch. β€” β€” Splinter Review
Updated for trunk.
Attachment #8347286 - Attachment is obsolete: true
Attachment #8362802 - Flags: ui-review+
Attachment #8362802 - Flags: review+
https://hg.mozilla.org/comm-central/rev/ea395ef92316
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: Thunderbird 30.0 → Thunderbird 29.0

Updated

5 years ago
Blocks: 962672
Depends on: 963731

Comment 47

5 years ago
I thought this change was an unintended  bug, and just now found out in the newsgroup that it was intentional. I know the decision has already been made, but I just want to say that - at least for Windows users - the UI is now worse than before. Users expect, and often need, a clear indication where an input is expected (i.e. a white field). They don't hunt around with the mouse looking for :hover events. This change will likely result in even more e-mails without the Subject filled in. It seems Mac-user developers are making decisions that don't benefit Windows users (i.e. the majority).
This whole "Mac Developers doing blank" trend is growing incredibly irritating and is simply false information. This design change was made on all platforms, following a new design not done by any particular platform.

As for your usability issue, a couple people have mentioned such a thing before, but I am purposely not changing the design as of now. I do not believe users will be unable to locate such text fields and no evidence has been given that suggests they do. However, if at a later time actual complaints have been made, then the design may be changed.

My reasoning here is that on a mail compose window, the idea of having long lines run across the window (where some fields are already filed in) is a clear enough indication as to it's purpose. Most people are familiar of filing in such "blanks" on mailing objects. In addition, many new and more modern mail applications and web apps embrace such a new style, including Airmail. Many designers I am friends with also follow a similar approach in their mockups. The design has undergone UX review twice and no concerns have been brought up. Therefore, at the present time, I am unwilling to modify the design. I believe we need this to land in beta and wait for (if any) bug reports before making other decisions.
FWIW, I don't like it *at all* either. I don't know about mac, but at least on linux and windows it is inconsistent with any other applications, and thunderbird itself, to have fields grayed out if you are supposed to fill them. AND they are grayed out even while blank. Like Peter, many any users will just think thunderbird is broken.

Comment 50

5 years ago
(:aceman from bug 960672 comment #8)
> I can't say I am happy with these changes (e.g. on Win XP the address widget
> and subject is solid grey as if uneditable even when it is already filled
> in, which is unintuitive). We have no such dynamic onmouseover UI in the
> rest of TB. But if you quickly update the rest of TB to be similar to this
> composer, it will not look so out of place:) Just please not more
> australification...

(In reply to rsx11m from comment #11)
> I'm not saying that I'm opposed to it, I was only voicing concerns that it
> may be confusing as it's an interesting but unexpected approach. ;-) Yes,
> the Subject line should be more clearly separated [...]

Comment 51

5 years ago
(In reply to :aceman from bug 960672 comment #8)
> I can't say I am happy with these changes

+1. I'm not entirely opposed to the general intention of this change either (which I think has some potential merits), but I still catch myself with a sigh of relief when toggling back to the current OS-conform layout with white input fields (on Windows at least). I've tried hard to convince myself the new layout is "better", but I can't quite say I've succeeded yet.

(In reply to Josiah Bruner [:JosiahOne] from comment #48)
> I do not believe users will be unable to locate such text fields and no evidence has
> been given that suggests they do. However, if at a later time actual
> complaints have been made, then the design may be changed.

Imho, all of the above comments from comment 47 onwards complain and provide initial evidence that we have an ux-discovery problem with this new design mainly driven by Josiah. Moreover, among the complainants are key TB contributors like :aceman, :mkmelin, and myself with at least considerable scepticism. I'm not sure what more complaints you want to wait for, and I strongly recommend that we follow the established path of discussing proposed changes in an open-minded and co-operative atmosphere while avoiding unilateral decisions with tendencies of ignoring and disparaging other opinions.

> My reasoning here is that on a mail compose window, the idea of having long
> lines run across the window (where some fields are already filed in) is a
> clear enough indication as to it's purpose. Most people are familiar of
> filing in such "blanks" on mailing objects.

The problem being, they don't look "blank" when unfilled, but "disabled" or unfillable by current OS-wide design conventions also found in the rest of TB (and I'm not very keen on trying to change that).

> In addition, many new and more
> modern mail applications and web apps embrace such a new style, including
> Airmail.

It would be good to have names, OS, and screenshots of those many so that other contributors can judge for themselves. I just checked Gmail webmailer which seems like a large enough example and their input fields in the area are white (but also cleaned up similar to what Josiah proposes; easier for them as they have the one-line-for-all addressing TB's still lacking).

> Many designers I am friends with also follow a similar approach in
> their mockups. The design has undergone UX review twice and no concerns have
> been brought up.

Uhm, wrong, sir (for the latter), and not an argument in itself (for the former).

> Therefore, at the present time, I am unwilling to modify
> the design.

!?

> I believe we need this to land in beta and wait for (if any) bug
> reports before making other decisions.

See my next comment.

Comment 52

5 years ago
Given the number of doubts and objections raised at this early stage *already*, I strongly recommend that we should try to find a broader consensus here and try out alternatives that address these objections. For that, I suggest that we go back to the original good intentions of Josiah's redesign:

(In reply to Josiah Bruner [:JosiahOne] from bug 867166 comment #0)
> Goal: To create a lighter, easier to use, more ascetically pleasant UI for
> the Compose window.

Evaluation of these for the current layout 

lighter:
- succeeds in removing several borders and perceived "edges"
- potentially fails as the new monolithic header area might even look heavier, especially with darker grey background

easier to use:
- don't see where this succeeds?
- fails (at least potentially) for the ux-discovery problems cited by various contributors above: input fields look disabled/unfillable in solid grey like uneditable background

more aesthetically pleasant:
- not sure how this is much different from "lighter" above
- question of taste, and tastes differ; if in doubt, usability definitely wins over aesthetics

In a nutshell, this is more of a "polish" type enhancement that is not required or very valuable as such and should be very sensitive to avoid introducing new ux-problems.


Brainstorming ideas for compromise:

A) On most windows versions (at least), fields for input are indicated by white background, and we should respect that. Josiah says, the header is a nightmare of heavyness (borders and edges). Both right - so then why don't we try this:
Remove all the borders and edges and have splitter below toolbar (as Josiah has well done), and just keep the plain white background of all editable fields (without any borders)? I'd love to see a screenshot of that, any volunteers?

B) IF there's a consensus that it's helpful to reduce saliency of input fields *after* they have been filled, we might additionally try this:
- white background only for recipient fields (including type toggles) and subject field only as long as they are empty (unfilled), to help ux-discovery and guide users visually into filling these
- after these fields are filled by user, they are arguably less likely to be edited again, so we might justify blending them back into the background (while keeping the current hover and focus effects), to make them less salient/distracting during composition.

I hasten to add that saliency has been a hotly contested topic in the past, in message reader's header, and users were very adamant against the "new design" experiments tried there - so pls consider the value of "once bitten, twice shy".
Fair enough.

I don't mind mocking up what they'd look like with a white background, those I believe it might look a little bizarre. 

However, perhaps another reason why this looks substantially better on OS X is that we are using a almost completely white background for the whole upper header section. Using the same color on all platforms may be helpful, and solve both the "darker header" and "grayed out textfields" problems. Although then we have the issue of dealing with Linux's themes (the dark ones).

We could try both approaches and see if either is preferred. Sounds to me like the "grayed out fields" is the real issue here, so both of those ideas should solve that.
BTW, let's move this discussion over to bug 867166 (The meta bug), since this bug is quite long and it actually works quite well on OS X (no objections to the OS X design as of yet).

Comment 55

5 years ago
(In reply to Thomas D. from comment #51)

> > Many designers I am friends with also follow a similar approach in
> > their mockups. The design has undergone UX review twice and no concerns have
> > been brought up.
> 
> Uhm, wrong, sir (for the latter), and not an argument in itself (for the
> former).

For the avoidance of doubt (language is so fuzzy...), what I meant is:
- 2 UX reviews: not an argument in itself (if not corroborated by detailed evaluation and addressing of identified potential ux problems)
- "no concerns have been brought up" - well, given that we work as a volunteer community, concerns are certainly existant and valid even when the don't come from official ui reviewers...
- I don't know about Josiah's designer friends

Comment 56

5 years ago
(In reply to Josiah Bruner [:JosiahOne] from comment #53)
> Fair enough.

Thanks for rapid reply.

> I don't mind mocking up what they'd look like with a white background, those
> I believe it might look a little bizarre.

Hopefully/certainly not more bizarre than the status quo these bugs want to improve on... ;)

> However, perhaps another reason why this looks substantially better on OS X
> is that we are using a almost completely white background for the whole
> upper header section. 

I agree with Josiah that a lighter color for the entire header background makes this look considerably less "disabled", as I commented elsewhere (might need someone to file a new bug). I'm not sure if it's enough to solve the ux-discovery problem. I wonder how plain white background for the whole header (except the toolbar?) would look, but I'm unsure which other ux-problems this might yield.

> We could try both approaches and see if either is preferred. Sounds to me
> like the "grayed out fields" is the real issue here, so both of those ideas
> should solve that.

Screenshots for any new approaches, especially on windows, would be helpful.
Thanks.
(In reply to Thomas D. from comment #55)
> (In reply to Thomas D. from comment #51)
> 
> > > Many designers I am friends with also follow a similar approach in
> > > their mockups. The design has undergone UX review twice and no concerns have
> > > been brought up.
> > 
> > Uhm, wrong, sir (for the latter), and not an argument in itself (for the
> > former).
> 
> For the avoidance of doubt (language is so fuzzy...), what I meant is:
> - 2 UX reviews: not an argument in itself (if not corroborated by detailed
> evaluation and addressing of identified potential ux problems)

Well the idea here is that one person who has been educated and works on Human Interaction had no issues, and another person had no issue, so that does definitely at least help strengthen the design. Not that it is the only voice in the matter.

> - "no concerns have been brought up" - well, given that we work as a
> volunteer community, concerns are certainly existant and valid even when the
> don't come from official ui reviewers...

I brought up my statement about the benefit of UI-reviewers above. There IS a reason we have such (few) people.

Perhaps I phrased it incorrectly, but my only reason for slightly disregarding other claims is because there was no real evidence other than speculation -> "Users will not...".

Of course, my second problem here (which you must understand, I'm sure) is that I quickly took all comments to mean: "The design does not work in anyway, and should be rolled-back" (Magnus and Peter gave me that idea for sure). I believe now the real issue is:

"The design may cause some users to not *easily* locate text fields on *linux* and *windows*", which I do agree with (or at least accept).

Also, just want to understand something here:

"Moreover, among the complainants are key TB contributors like :aceman, :mkmelin"

Just making sure you realize I am well aware of who our active contributors are, and also hope you realize (perhaps you do) that I am one of them. (I'm a theme peer). Just clearing that up, but perhaps I read into that statement. Let me know.


Anyway, I commented here: https://bugzilla.mozilla.org/show_bug.cgi?id=867166#c7, where we can try new ideas. (I also uploaded what things look like on OS X, which seems to work well) I want to have mockups up later today if possible.

Comment 58

5 years ago
BTW, any volunteers for Bug 440377? - Redesign recipients address fields (To, Cc, Bcc) as single-line input fields for multiple comma separated addresses - not one line/row per address

That would go a long way to clean up the header and free some vertical screen real estate while easier to use (not only because almost all other mailers works that way)...

Josiah, there's a minor design glitch I see on WinXP, namely after starting new composition, there's a vertical scrollbar showing in the header which shouldn't by default (needs 2 or 3 more vertical pixels in the header area to go away; might be due to age-old design bugs in this corner because the scrollbar comes and goes in odd ways depending on vertical resizing).
(In reply to Thomas D. from comment #58)
> BTW, any volunteers for Bug 440377? - Redesign recipients address fields
> (To, Cc, Bcc) as single-line input fields for multiple comma separated
> addresses - not one line/row per address
> 
> That would go a long way to clean up the header and free some vertical
> screen real estate while easier to use (not only because almost all other
> mailers works that way)...

Ha, actually yes. That's already on my todo list. I've started minimal work on it already. My goal is to get that into TB 31, (since I think that is more important than a dark theme)

> 
> Josiah, there's a minor design glitch I see on WinXP, namely after starting
> new composition, there's a vertical scrollbar showing in the header which
> shouldn't by default (needs 2 or 3 more vertical pixels in the header area
> to go away; might be due to age-old design bugs in this corner because the
> scrollbar comes and goes in odd ways depending on vertical resizing).

Yes, I believe that was mentioned in the Windows bug. If I remember correctly, Richard said it happened before the change as well. But updating the height might be all it takes.
You need to log in before you can comment on or make changes to this bug.