988971 - Replace Classic with "Sandstone" skin as Standard skin

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement, P1)

Description

[David Lawrence [:dkl]]

The default skin on bugzilla.mozilla.org is called Sandstone due to the similarities to the Sandstone Mozilla.org theme in use at the time. Several people have requested that the skin be made available in the upstream Bugzilla code so this bug is about tracking that.

Currently the following things will need to be taken into account when porting.

1. All of the CSS is located in skins/contrib/Mozilla/global.css and will need to be broken out into individual files such as global.css, enter_bug.css, show_bug.css, buglist.css, etc.
2. Changes are needed in to the global/header.html.tmpl specifically for the Sandstone skin currently. We either need to port those changes as-is or make changes to the standard header template that allows for all skins to share the same HTML and have the Sandstone skin achieve the same look using pure CSS and/or jscript.
3. The footer needs much improvement. One of the eventual goals of the Mozilla skin is to convert the save queries and other actions into vertical lists or drop down menus to make the footer cleaner.

If someone wants to take this one that would be great, otherwise BMO devels will be able to work on this after our other goals are completed.

dkl

Comment 1

[David Lawrence [:dkl]]

For reference:

BMO's header:
http://git.mozilla.org/?p=webtools/bmo/bugzilla.git;a=blob;f=template/en/default/global/header.html.tmpl;h=c5b2c126afbd001dac04d2778291f0cf2320ad93;hb=refs/heads/4.2 [github]

CSS and images:
http://git.mozilla.org/?p=webtools/bmo/bugzilla.git;a=tree;f=skins/contrib/Mozilla;h=111f782a4e71572ed48e4d4c3ca34c0475d47ab6;hb=refs/heads/4.2 [github]

Comment 2

[Mark Côté [:mcote]]

We'd like to see this in Bugzilla 5.0.

Comment 3

[Frédéric Buclin]

(In reply to David Lawrence [:dkl] from comment #0)
> 1. All of the CSS is located in skins/contrib/Mozilla/global.css and will
> need to be broken out into individual files such as global.css,
> enter_bug.css, show_bug.css, buglist.css, etc.

enter_bug.css and show_bug.css no longer exist in 5.0. This should make things easier. :)


> 3. The footer needs much improvement. One of the eventual goals of the
> Mozilla skin is to convert the save queries and other actions into vertical
> lists or drop down menus to make the footer cleaner.

Using a drop down menu is bug 324402, which I filed 8 years ago (wow!).

Comment 4

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #3)
> (In reply to David Lawrence [:dkl] from comment #0)
> > 1. All of the CSS is located in skins/contrib/Mozilla/global.css and will
> > need to be broken out into individual files such as global.css,
> > enter_bug.css, show_bug.css, buglist.css, etc.
> 
> enter_bug.css and show_bug.css no longer exist in 5.0. This should make
> things easier. :)

Ah good to know.
 
> > 3. The footer needs much improvement. One of the eventual goals of the
> > Mozilla skin is to convert the save queries and other actions into vertical
> > lists or drop down menus to make the footer cleaner.
> 
> Using a drop down menu is bug 324402, which I filed 8 years ago (wow!).

Yeah after writing this I thought that 3. may be best as a separate bug and can be used by any skin. So lets not do that part as part of this bug and get bug 324402 rebooted and work on that separately.

dkl

Comment 5

[u430950]

Attachment: patch.diff

Patch consists of changes necessary to bring Sandstone functionality to all skins.

Comment 6

[u430950]

Attachment: Sandstone.tar.gz

Sandstone skin

Comment 7

[u430950]

Attachment: sandstonev2.diff

Cleaned up files and added the Sandstone contrib folder to the patch.

Comment 8

[Frédéric Buclin]

All CSS files must have a MPL 2.0 license.

Comment 9

[u430950]

Attachment: sandstonev3.diff

Added MPL 2.0 license

Comment 10

[:glob ✱]

Comment on attachment 8479134 [details] [diff] [review]
sandstonev3.diff

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

this looks fantastic :)  nice work ryan.
i've tested it in IE 11.

- there's some commented out code which should be removed
- trailing whitespace also needs to be removed

- the default skin should be changed from dusk to sandstone
  - when the preference is created: Bugzilla/Install.pm line 70
  - change any existing sites currently using dusk to sandstone (sites using skins other than dusk shouldn't be touched)

::: skins/contrib/Sandstone/bug.css
@@ +55,5 @@
> +body[class*=bz_group_] {
> +    background-color: inherit;
> +    background-image: url(index/noise.png), -moz-linear-gradient(#d7d7ff, #f0f0ff 400px);
> +    background-image: url(index/noise.png), -webkit-linear-gradient(#d7d7ff, #f0f0ff 400px);
> +    background-image: url(index/noise.png), linear-gradient(#d7d7ff, #f0f0ff 400px);

index --> global (as per other comment about renaming the index subdir)

::: skins/contrib/Sandstone/global.css
@@ +9,5 @@
> +@font-face {
> +  font-family: 'Open Sans';
> +  font-style: normal;
> +  font-weight: 400;
> +  src: local('Open Sans'), local('OpenSans'), url(index/opensans.woff) format('woff');

all files referenced in global.css belong in a "global" subdirectory.
eg. url(global/opensans.woff)

@@ +150,5 @@
> +
> +#title {
> +    width: 150px;
> +    font-size: 120%;
> +}

the #title block doesn't look correctly aligned with the "Home" link.

i suggest changing the padding in #titles to
  padding: 10px 10px 0 10px;
and adding padding to #title:
  padding: 0px 0px 0px 5px;

@@ +191,5 @@
> +
> +#enter_bug	{ background: url(index/bugzilla-papericon.png) no-repeat; }
> +#query		{ background: url(index/bugzilla-magnifier.png) no-repeat; }
> +#account	{ background: url(index/bugzilla-person-alternate.png) no-repeat; }
> +#help		{ background: url(index/bugzilla-questionmark2.png) no-repeat; }

..except for these files, which actually belong in the index subdir (to match the standard skin's layout)

::: template/en/default/global/common-links.html.tmpl
@@ +40,4 @@
>    [% Hook.process('action-links') %]
>  
>    [% IF user.login %]
> +    <li class="dropdown">

this should only be displayed in the header; there's no need to duplicate it in the footer, and it displays off the page.

ie. only display when qs_suffix is "_top"

@@ +44,5 @@
> +      <span class="anchor">[% user.login FILTER html %]</span>
> +      <ul>
> +        [% IF user.showmybugslink %]
> +          [% filtered_username = user.login FILTER uri %]
> +          <li><a href="[% Param('mybugstemplate').replace('%userid%', filtered_username) %]">My [% terms.Bugs %]</a></li>

these <li>'s can be a little bit hard to hit; i think a little extra padding here would help.

Comment 11

[u430950]

Attachment: sandstonev4.diff

All the changes requested in comment 10 have been corrected in this patch.

Comment 12

[Frédéric Buclin]

Comment on attachment 8504134 [details] [diff] [review]
sandstonev4.diff

>diff --git a/skins/contrib/Dusk/global.css b/skins/contrib/Dusk/global.css

There is no need to duplicate rules which are exactly the same in standard/*.css. Due to cascading, Dusk/*.css should only contain new or changed rules. This also makes maintaince much easier.

The same comment applies to Standstone/*.css.

Comment 13

[David Lawrence [:dkl]]

Too late for 5.0. Targetting for 6.0.

Comment 14

[u430950]

Attachment: sandstonev5.diff

Updated with removal of redundant .css changes.

Comment 15

[u430950]

Attachment: sandstonev6.diff

Forgot the change to the Bugzilla/Install.pm file in previous patch

Comment 16

[Frédéric Buclin]

Could you attach a tar.gz archive with all binary files in it (PNG, WOFF)? I know they are in attachment 8433436 [details], but this archive also contains CSS files which are already part of the patch.

Comment 17

[Frédéric Buclin]

... and it seems this archive doesn't have all the PNG and WOFF files, at least not in the right place. In your patch, there are both index/noise.png and global/noise.png. Is that intentional?

Comment 18

[Frédéric Buclin]

Comment on attachment 8548950 [details] [diff] [review]
sandstonev6.diff

>+++ b/template/en/default/global/common-links.html.tmpl

>+    [% IF qs_suffix == "_top" %]
>+        <li class="dropdown">

I'm not a fan to see these links being removed from the page footer. It's painful to have to jump to the top of the page. I see no reason to remove them from there. And I don't see why some links would remain (New - Search - Browse) while some others wouldn't (Administration - Preferences - Requests). Because the latter ones are used less often?

Also, why is the search box in the page footer smaller than the one in the page header?

Comment 19

[Greg Smethells]

It has been 4.5 years since the Bugzilla Pretty contest occurred:

https://wiki.mozilla.org/Bugzilla:Pretty

Comment 20

[Gervase Markham [:gerv]]

It's sad that this has got stuck :-| f1sh: are you interested in finishing it up, if we can find a reviewer?

Gerv

Comment 21

[u430950]

I'm more than willing to finish this bug once we have a reviewer.

Comment 22

[Gervase Markham [:gerv]]

We discussed this in the last Bugzilla meeting; there is a support cost to carrying more skins in core, and we aren't sure that we want Sandstone itself, as opposed to pinching its best ideas and incorporating them into an existing skin.

Gerv

Comment 23

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #22)
> we aren't sure that we want Sandstone itself

I'm not sure what Sandstone is these days, but I would suggest to implement the "experimental UI" from BMO. I turned it on a few minutes ago, and I already love it. :) (yes I know, I don't say such things very frequently)

And personally, I would consider it as the 6th (and final) goal for 6.0 (see bug 1234917 comment 0 for the other 5 goals).


(In reply to Ryan Wilson [:f1sh] from comment #21)
> I'm more than willing to finish this bug once we have a reviewer.

I can review it.

Comment 24

[Gervase Markham [:gerv]]

The current plan of record here is that f1sh will port Sandstone to core, overwriting Classic (although perhaps retaining the name) and rebase Dusk on top of it. So we will then have two skins, Sandstone and Dusk, and the "experimental UI" from BMO can be ported to trunk as it works on top of Sandstone.

f1sh: can you confirm that I have outlined this correctly?

Gerv

Comment 25

[Dylan Hardison [:dylan] (he/him)]

(In reply to Frédéric Buclin from comment #23)
> (In reply to Gervase Markham [:gerv] from comment #22)
> > we aren't sure that we want Sandstone itself
> 
> I'm not sure what Sandstone is these days, but I would suggest to implement
> the "experimental UI" from BMO. I turned it on a few minutes ago, and I
> already love it. :) (yes I know, I don't say such things very frequently)
> 
> And personally, I would consider it as the 6th (and final) goal for 6.0 (see
> bug 1234917 comment 0 for the other 5 goals).
 This is what I was working on in my modal port branch, BTW. https://github.com/dylanwh/bugzilla/tree/modal?files=1

Comment 26

[u430950]

(In reply to Gervase Markham [:gerv] from comment #24)
> The current plan of record here is that f1sh will port Sandstone to core,
> overwriting Classic (although perhaps retaining the name) and rebase Dusk on
> top of it. So we will then have two skins, Sandstone and Dusk

Yes, this is my plan. I'm currently writing as if it is Classic, but it should be as easy as renaming a couple of default fields if we want to be rid of the Classic name.

Comment 27

[u430950]

Attachment: standard-sandstone.diff

Here is the first patch moving Sandstone into the standard skin.

Comment 28

[u430950]

Attachment: standard-sandstone-v2.diff

Noticed some missing files and odd header behavior. Submitting a new patch.

Comment 29

[u430950]

Comment on attachment 8742993 [details] [diff] [review]
standard-sandstone-v2.diff

Removing the review request, as a new patch is forthcoming with changes introduced in bug 1167270 comment 5

Comment 30

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8742993 [details] [diff] [review]
standard-sandstone-v2.diff

items with class=throw_error are a little hard to read. I haven't noticed any other issues. Can you fix the colors on those?

Comment 31

[u430950]

I will have a new patch ready next week.

Comment 32

[GitHub Autolander Bot]

Attachment: [bugzilla] thef1sh:sandstone > bugzilla:master

Comment 33

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8784992 [details] [review]
[bugzilla] thef1sh:sandstone > bugzilla:master

r=dyan

Comment 34

[Dylan Hardison [:dylan] (he/him)]

To github.com:bugzilla/bugzilla.git
   b9c4001..4da53d8  master -> master