Open Bug 1014208 Opened 6 years ago Updated 3 years ago

Updated InContent Preferences Design Based on Project Chameleon

Categories

(Firefox :: Preferences, defect)

defect
Not set
normal

Tracking

()

Tracking Status
relnote-firefox --- 38+

People

(Reporter: mmaslaney, Unassigned)

References

(Depends on 2 open bugs)

Details

(Keywords: meta, Whiteboard: [Australis])

Attachments

(6 files, 4 obsolete files)

Attached image Preferences - General@2x.png (obsolete) —
Attached, the updated design for the inContent Preferences, based on the Project Chameleon initiative. We can ignore the global account navigation you see at the top, which will be future work.

I'll provide additional mocks and specs within the next few weeks.

John Gruen and I have started to document our UI here (this is WIP): http://people.mozilla.org/~jgruen/chameleon/#nav0
John, can you please update this bug with the updated Chameleon design that you plan on publishing tomorrow? Please include a zip attachment of the specs as well for posterity.
Flags: needinfo?(jgruen)
Assignee: nobody → ntim007
Status: NEW → ASSIGNED
(In reply to mmaslaney from comment #0)
> John Gruen and I have started to document our UI here (this is WIP):
> http://people.mozilla.org/~jgruen/chameleon/#nav0

Not sure whether to file this as an issue over at Github or not, but I'll post it here. The typography section seems to have forgotten Linux.
(In reply to Paul [pwd] from comment #4)
> (In reply to mmaslaney from comment #0)
> > John Gruen and I have started to document our UI here (this is WIP):
> > http://people.mozilla.org/~jgruen/chameleon/#nav0
> 
> Not sure whether to file this as an issue over at Github or not, but I'll
> post it here. The typography section seems to have forgotten Linux.

Linux coming!
Flags: needinfo?(jgruen)
Attached image chameleon_checkbox.svg
Adding updated SVG checkbox
Depends on: 1017555
Depends on: 1017631
Attached file inContent-assets.zip
Updated InContent Assets
Spec in progress.
Depends on: 1017722
Michael, do you think you can define the menu list dropdown items styling ? Thanks :)
Flags: needinfo?(mmaslaney)
Depends on: 1017725
Assignee: ntim007 → nobody
Depends on: 1018717
Status: ASSIGNED → NEW
Depends on: 1018796
Tim, John Gruen is currently working on the live spec. More to come...
Flags: needinfo?(mmaslaney)
We will be using Fira 3.1 for all inContent UI typography, specifically the Light weight for headers and the Book and Bold weights for body copy. John will be capturing this in the spec.
Attached image Preferences-general.png
Updated:

• Header Icon has been removed, as it seemed repetitive.
• Aligned to the updated grid
Attachment #8426496 - Attachment is obsolete: true
Attached patch updateInContent.patch (obsolete) — Splinter Review
This is a first try. It needs bug 1020286 applied first.

I removed the header icons and changed the background color. I also added the header bottom border except on advanced pane there  with the tabs top border to much lines are together.

On General pane I tried to align all on the grid but failed in downloads area. I think this is because of the radiogrid which breaks the grid.

Jared, do you know how I can implement the safeTo radio line to this global grid? I had already to remove the groupboxes to apply the grid to all Startup group lines. But removing the radiogroup would break it's functionality.

Michael, I created a try build[1] for easier testing. What do you think about the spacing, colors etc.?

[1] https://tbpl.mozilla.org/?tree=Try&rev=2e60bda034cb
Attachment #8440413 - Flags: feedback?(mmaslaney)
Attachment #8440413 - Flags: feedback?(jaws)
Fira Sans is not used anymore for in-content (bug 1007629). It should be reimplemented if it is to be used.
Depends on: 1025687
Attached patch updateInContent.patch (obsolete) — Splinter Review
This is a different approach with setting the with of the first element in the rows through a locale entry. This also works in radiogroups. I also fixed the font size on Linux menulists. I'm trying to ask for reviews to go further with this.

The grid alignment is only done on main page as of the other pages I don't see how this could be done. Michael, please can you add mockups of the other pages, if needed? And can they be done in followup bugs?

Again for easier review the try builds: https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/richard.marti@gmail.com-305a9e190903
Assignee: nobody → richard.marti
Attachment #8440413 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8440413 - Flags: feedback?(mmaslaney)
Attachment #8440413 - Flags: feedback?(jaws)
Attachment #8444093 - Flags: ui-review?(mmaslaney)
Attachment #8444093 - Flags: review?(jaws)
Attached image Comparison of new and old UI (obsolete) —
New is on the left, old is on the right. As you can see, it caused 2 regressions for me : the buttons text are not overflowed correctly, there is now a new vertical scrollbar (and my display is quite big (1600 x 900).

For the vertical scrollbar, we can investigate later for making the page responsive. Bug 1026679 can be used for this.
Comment on attachment 8444093 [details] [diff] [review]
updateInContent.patch

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

::: browser/components/preferences/in-content/main.xul
@@ +103,3 @@
>    <hbox align="center">
> +    <label class="align-end"
> +           style="min-width: &mainPane.width;;"

Why is this necessary? I don't see why locales will want to change the width here, we should just let the content flow as normal.

@@ +138,5 @@
>               preference="browser.startup.homepage"/>
>    </hbox>
> +  <separator class="thin"/>
> +  <hbox align="center">
> +    <hbox style="min-width: &mainPane.width;;"/>

This is a pretty bad hack.

@@ +139,5 @@
>    </hbox>
> +  <separator class="thin"/>
> +  <hbox align="center">
> +    <hbox style="min-width: &mainPane.width;;"/>
> +    <hbox flex="1">

This <hbox> seems unnecessary. Can you move the buttons to be direct children of the <hbox align="center"> above?

::: browser/themes/linux/preferences/in-content/preferences.css
@@ +85,5 @@
>    margin-bottom: 11px;
>  }
> +
> +#chooseFolder {
> +  -moz-margin-start: -4px;

Why is this needed?
Attachment #8444093 - Flags: review?(jaws) → review-
(In reply to Jared Wein [:jaws] [Away for most of July] (please needinfo? me) from comment #21)
> Comment on attachment 8444093 [details] [diff] [review]
> updateInContent.patch
> 
> Review of attachment 8444093 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/linux/preferences/in-content/preferences.css
> @@ +85,5 @@
> >    margin-bottom: 11px;
> >  }
> > +
> > +#chooseFolder {
> > +  -moz-margin-start: -4px;
> 
> Why is this needed?
To make the file input and button stick together.
(In reply to Jared Wein [:jaws] [Away for most of July] (please needinfo? me) from comment #21)
> Comment on attachment 8444093 [details] [diff] [review]
> updateInContent.patch
> 
> Review of attachment 8444093 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/components/preferences/in-content/main.xul
> @@ +103,3 @@
> >    <hbox align="center">
> > +    <label class="align-end"
> > +           style="min-width: &mainPane.width;;"
> 
> Why is this necessary? I don't see why locales will want to change the width
> here, we should just let the content flow as normal.

I ended with this to align all textfields, menulists and buttons like on the mockup from Michael. In previous patch (attachment 8440413 [details] [diff] [review]) I tried it with a grid and had already to remove the groupboxes to make the grid working but failed on the radiogroup #saveWhere there the grid not worked.

The localized width is to adapt to the different length of the localized labels. I know it's hackish but I saw no other solution. If you know one, I'm happy to use it.

> @@ +139,5 @@
> >    </hbox>
> > +  <separator class="thin"/>
> > +  <hbox align="center">
> > +    <hbox style="min-width: &mainPane.width;;"/>
> > +    <hbox flex="1">
> 
> This <hbox> seems unnecessary. Can you move the buttons to be direct
> children of the <hbox align="center"> above?

Yes, this is a remnant from grid patch.

> ::: browser/themes/linux/preferences/in-content/preferences.css
> @@ +85,5 @@
> >    margin-bottom: 11px;
> >  }
> > +
> > +#chooseFolder {
> > +  -moz-margin-start: -4px;
> 
> Why is this needed?

To attach the button directly to the textfield. OS X uses a different value than Linux and Windows.
(In reply to Tim Nguyen [:ntim] from comment #22)
> > > +
> > > +#chooseFolder {
> > > +  -moz-margin-start: -4px;
> > 
> > Why is this needed?
> To make the file input and button stick together.

That's the surface answer, but is there a deeper reasoning as to why this is needed?

Can you try placing the the elements right next to each other in the markup (<filefield/><button/> instead of <filefield/> <button/>).
(In reply to Jared Wein [:jaws] [Away for most of July] (please needinfo? me) from comment #24)
> (In reply to Tim Nguyen [:ntim] from comment #22)
> > > > +
> > > > +#chooseFolder {
> > > > +  -moz-margin-start: -4px;
> > > 
> > > Why is this needed?
> > To make the file input and button stick together.
> 
> That's the surface answer, but is there a deeper reasoning as to why this is
> needed?
> 
> Can you try placing the the elements right next to each other in the markup
> (<filefield/><button/> instead of <filefield/> <button/>).

No change, the filefield and the button have margins around them and the negative margin on button is to revert this.
(In reply to Richard Marti (:Paenglab) from comment #23)
> (In reply to Jared Wein [:jaws] [Away for most of July] (please needinfo?
> me) from comment #21)
> > Comment on attachment 8444093 [details] [diff] [review]
> > updateInContent.patch
> > 
> > Review of attachment 8444093 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: browser/components/preferences/in-content/main.xul
> > @@ +103,3 @@
> > >    <hbox align="center">
> > > +    <label class="align-end"
> > > +           style="min-width: &mainPane.width;;"
> > 
> > Why is this necessary? I don't see why locales will want to change the width
> > here, we should just let the content flow as normal.
> 
> I ended with this to align all textfields, menulists and buttons like on the
> mockup from Michael. In previous patch (attachment 8440413 [details] [diff] [review]
> [review]) I tried it with a grid and had already to remove the groupboxes to
> make the grid working but failed on the radiogroup #saveWhere there the grid
> not worked.
> 
> The localized width is to adapt to the different length of the localized
> labels. I know it's hackish but I saw no other solution. If you know one,
> I'm happy to use it.
width: -moz-fit-content ?
(In reply to Tim Nguyen [:ntim] from comment #26)

> width: -moz-fit-content ?

I don't see how this could imitate a grid. Please can you give a example?
(In reply to Richard Marti (:Paenglab) from comment #27)
> (In reply to Tim Nguyen [:ntim] from comment #26)
> 
> > width: -moz-fit-content ?
> 
> I don't see how this could imitate a grid. Please can you give a example?

I was thinking of a method, but that didn't work. You could try a table layout or a column layout. There's currently a spec in progress for CSS Grid, but only IE supports it. Also, I *think* the preferences window has the layout you want to achieve, so you might want to check it.
I tried the grid in previous patch but this failed in downloads groupbox with it's radiogroup. Maybe it's possible to remove the radiogroup and then imitate the group with the two radios through JS. But would this be okay? And I'm to much a JS beginner to create such JS.
(In reply to Richard Marti (:Paenglab) from comment #29)
> I tried the grid in previous patch but this failed in downloads groupbox
> with it's radiogroup. Maybe it's possible to remove the radiogroup and then
> imitate the group with the two radios through JS. But would this be okay?
> And I'm to much a JS beginner to create such JS.

You could simply calculate the main pane width with JS then apply it.
I don't need the main pane width. I need the "When Nightly starts:", "Home Page:" and "Save files to" have the same width, then the menulists, textboxes and buttons behind them will start at the same position like in a grid. I can't count on "When Nightly starts:" is the longest and use it's width. I other locales a other string of this three could be the longest.
Needinfo Jared to look into it.
Flags: needinfo?(jaws)
(In reply to Richard Marti (:Paenglab) from comment #31)
> I don't need the main pane width. I need the "When Nightly starts:", "Home
> Page:" and "Save files to" have the same width, then the menulists,
> textboxes and buttons behind them will start at the same position like in a
> grid. I can't count on "When Nightly starts:" is the longest and use it's
> width. I other locales a other string of this three could be the longest.
You should calculate the longest label, then use it. I'll create a JSFiddle demo if you want.
You should not use JS to do the layout. Attachment 8439539 [details] doesn't look _that_ fancy. If standard XUL doesn't work for you here, you're probably not using it the right way.
(In reply to Dão Gottwald [:dao] from comment #35)
> If standard XUL doesn't work for you here, you're probably not
> using it the right way.

This is what I tried:

<grid >
  <columns>
    <column/>
    <column flex="1"/>
  </columns>
  <rows>
    some other <row/> which are working
    <row>
      <caption><label></label></caption>
    </row>
    <radiogroup>
      <row>
        <radio id="saveTo"/>
        <hbox flex="1">
          <filefield/><button/>
        </hbox>
      </row>
      <row>
        <radio/>
      </row>
    </radiogroup>
  </rows>
</grid>

The radiogroup breaks the grid. How can I align the radio in radiogroup to the global grid?
(In reply to Richard Marti (:Paenglab) from comment #36)
Have you tried wrapping the radiogroup with a row, then removing the rows inside the radiogroup ?
(In reply to Tim Nguyen [:ntim] from comment #37)
> (In reply to Richard Marti (:Paenglab) from comment #36)
> Have you tried wrapping the radiogroup with a row, then removing the rows
> inside the radiogroup ?

This doesn't work. Then the whole radiogroup is in the first column.
(In reply to Richard Marti (:Paenglab) from comment #36)
> (In reply to Dão Gottwald [:dao] from comment #35)
> > If standard XUL doesn't work for you here, you're probably not
> > using it the right way.
> 
> This is what I tried:
> 
> <grid >
>   <columns>
>     <column/>
>     <column flex="1"/>
>   </columns>
>   <rows>
>     some other <row/> which are working
>     <row>
>       <caption><label></label></caption>
>     </row>
>     <radiogroup>
>       <row>
>         <radio id="saveTo"/>
>         <hbox flex="1">
>           <filefield/><button/>
>         </hbox>
>       </row>
>       <row>
>         <radio/>
>       </row>
>     </radiogroup>
>   </rows>
> </grid>
> 
> The radiogroup breaks the grid. How can I align the radio in radiogroup to
> the global grid?
According to mdn, your example should work, but without the rows wrapping the content. See : https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Tutorial/Grids#Column_Spanning
(In reply to Richard Marti (:Paenglab) from comment #32)
> Needinfo Jared to look into it.

It looks like this has already been answered.
Flags: needinfo?(jaws)
(In reply to Jared Wein [:jaws] [Away for most of July] (please needinfo? me) from comment #40)
> (In reply to Richard Marti (:Paenglab) from comment #32)
> > Needinfo Jared to look into it.
> 
> It looks like this has already been answered.

Hmm, where is the answer which works? I tried all and don't found a solution.
Depends on: 1036090
Depends on: 1038288
Depends on: 1038291
Changing this bug to meta with the lot of blocking bugs.
Keywords: meta
Comment on attachment 8444093 [details] [diff] [review]
updateInContent.patch

A meta bug shouldn't have patches.
Attachment #8444093 - Attachment is obsolete: true
Attachment #8444093 - Flags: ui-review?(mmaslaney)
Depends on: 1052318
The plan here from in-content prefs triage is to prune through the list of the bugs blocking this one and put the ones that we feel are specifically blocking shipping the in-content prefs as blocking the 'ship-incontent-prefs' bug. They can also stay blocking this bug. After that task is completed, this bug can be removed from the blocking list of the 'ship-incontent-prefs' bug.
Unassign because it's a meta bug.
Assignee: richard.marti → nobody
Status: ASSIGNED → NEW
Updated mocks for the InContent UI Prefs: http://invis.io/P61RQ2CEB
No longer depends on: 1013693
No longer depends on: 1013695
No longer depends on: 1013706
No longer depends on: 1013701
No longer depends on: 1013703
Depends on: 1111332
Now that most of the bugs that are blocking this bug have been fixed, we can remove this as a blocker for shipping in-content prefs.

The remaining open bugs as of today (1013714, 1025687, 1038291) are minor polish, potential new font but unlikely, and re-layout of a section of the general preferences (close to landing). None of these are severe enough to stop us from blocking shipping though.
No longer blocks: ship-incontent-prefs
Depends on: 1130007
No longer depends on: 1130007
Can you make it so that selecting a menu item does not cause a typing indicator[1] to appear? I also think it would feeld more "native" if you cannot select the text of the menu item.[2]

[1]: http://i.imgur.com/J2SRH1p.png
[2]: http://i.imgur.com/pJYXEg1.png
(In reply to Stanzilla from comment #48)
> Can you make it so that selecting a menu item does not cause a typing
> indicator[1] to appear? I also think it would feeld more "native" if you
> cannot select the text of the menu item.[2]
> 
> [1]: http://i.imgur.com/J2SRH1p.png
> [2]: http://i.imgur.com/pJYXEg1.png

Can you file a bug please ? Thanks.
Depends on: 1135508
(In reply to Tim Nguyen [:ntim] from comment #49)
> (In reply to Stanzilla from comment #48)
> > Can you make it so that selecting a menu item does not cause a typing
> > indicator[1] to appear? I also think it would feeld more "native" if you
> > cannot select the text of the menu item.[2]
> > 
> > [1]: http://i.imgur.com/J2SRH1p.png
> > [2]: http://i.imgur.com/pJYXEg1.png
> 
> Can you file a bug please ? Thanks.

There you go https://bugzilla.mozilla.org/show_bug.cgi?id=1135508
Depends on: 1136645
Depends on: 1136671
Added to the release notes with "New tab-based Preferences" as wording.
Some documentations or blog post would be nice!
Attachment #8444123 - Attachment is obsolete: true
Depends on: 1148749
You need to log in before you can comment on or make changes to this bug.