Closed Bug 259723 Opened 20 years ago Closed 17 years ago

Alternative Bugzilla CSS

Categories

(Bugzilla :: User Interface, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: mschrag, Assigned: Wurblzap)

References

()

Details

(Whiteboard: relnote comment 67)

Attachments

(3 files, 10 obsolete files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 MultiZilla/1.5.0.3l Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 MultiZilla/1.5.0.3l Bugzilla is an awesome product, but the UI leaves a lot to be desired (aqua and purple tabs? what's their story?). I'm attaching a replacement UI patch against 2.19/HEAD. There are quite a few pages to be done still, but the core pages are converted. Reproducible: Always Steps to Reproduce:
Attached image screenshot of UI (obsolete) —
the patch is missing global/useful-links-actions.html.tmpl
Attached patch adds missing file (obsolete) — Splinter Review
Sorry about that -- I forgot I can't cvs add new files w/o write access. This one is a normal diff instead of a cvs diff; patch -p1 it since it's a folder up.
Attachment #159059 - Attachment is obsolete: true
can we coordinate this with what's being done as part of bug 253449? It'd be great if this kind of thing could be done with only css changes :)
That would be great ... The bulk of this patch actually is CSS changes. The other chunk is mostly converting existing templates to take better advantage of CSS (consistently class'ing things, etc). There were a /few places/ where I had to sacrifice better CSS for compatibility with IE by setting things up into tables. For instance, my first rev used display: table-cell/table/etc; which IE is pretty good at screwing up apparently. But I did convert a ton of <td><b>'s over to <th>'s inside of named higher level <div>'s, etc.
very nice :) i like, lots. when the useful-links-actions are in the header, the "find bug #" doesn't work (there's no <form>) the patch contains tabs, these should be converted to spaces. i'm also not a fan of the lowercase text transformation, as it leaves the page with a mixure of titlecase and lowercase. the colour for <a> is overriding the colour for the btn class, so <span class="btn"><a href="#">View Bug Activity</a></span> doesn't render the text in white, making it hard to read.
Thanks! That last patch actually has a fix for that form button in it too (I noticed this after I submitted the original patch and fixed it in our local version) ... The form tag really needs to move inside that tmpl file, but I just did a quick fix and wrapped it in banner as well. Re: tabs -- yeah my bad. I tend to not notice because my tabs match bugzilla's spacing. I'll fix that if/when I submit a followup patch. Re: the lowercase, I guess that's the beauty of CSS that you can just change that. I really wish you could do dependencies/variables in CSS so you could just change it one place (and so I didn't need to redefine that button look several times), but there are only 3 "lowercase"'s in there so it's not to bad of a change. You can also do a .label:after { content: ": "; } if you want the colons back at the end of the label names. Re: color -- Are you referring to the light brown buttons in the knob.html.tmpl on the view bug page? If so, there's actually a ".nestedBox .btn" style that has its link color set to black explicitly. With that secondary brown color, white was actually hard to read also, so black seemed the lesser of evils. I need to try alternatives. Feel free to muck around w/ global.css and attach if you get some better color combos going ... This was just a first pass to get it looking a little better than stock.
my bad -- 'patch' did a bad job of updating global.css, which resulted in a lot of styles being missed.
Depends on: 264511
Attached file modified global.css (obsolete) —
here's a different global.css basically i split mike's patch into two -- one to cover the template changes (bug 264511) and one for the css changes (this one). the original template changes were large and didn't downgrade very well, so i redid the changes keeping them as minimal as possible while still trying to keep the feel of mike's change. there's places that i've had to compromise, especially in making hrefs look like buttons, but i think overall it still looks good.
Attached image home page screenshot (obsolete) —
Attached image bug screenshot
Nice changes ... I agree it's hard to merge the original versions.
Attachment #162191 - Attachment is obsolete: true
Attached file update global.css (obsolete) —
Assignee: myk → bugzilla
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Alternative Bugzilla UI → Alternative Bugzilla CSS
For demonstration purposes, I took the liberty to take the skin here to bug 321556 :)
there's more work happening on bug 321556 with this skin, so i'll mark this as a duplicate. *** This bug has been marked as a duplicate of 321556 ***
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → DUPLICATE
Blocks: 321556
Status: RESOLVED → REOPENED
No longer depends on: 264511
Resolution: DUPLICATE → ---
No longer blocks: 321556
Depends on: 321556
Attached file Mike's Skin (obsolete) —
This is Mike's skin, adapted to work with bug 322693 and bug 321556.
Assignee: bugzilla → wurblzap
Status: REOPENED → ASSIGNED
(In reply to comment #17) > Created an attachment (id=211458) [edit] > Mike's Skin > > This is Mike's skin, adapted to work with bug 322693 and bug 321556. > The attachment is listed as a patch but it looks a lot like a binary file...
Attachment #211458 - Attachment is patch: false
Attachment #211458 - Attachment mime type: text/plain → text/binary
Comment on attachment 211458 [details] Mike's Skin this is a zipped set of css
Comment on attachment 211458 [details] Mike's Skin Right, sorry. I checked the "patch" tick mark by habit.
Attachment #211458 - Attachment mime type: text/binary → application/x-compressed-tar
QA Contact: mattyt-bugzilla → default-qa
Attached file Mike's Skin (obsolete) —
Update to the demonstration skin, to work with bug 321556.
Attachment #211458 - Attachment is obsolete: true
Attachment #240550 - Attachment mime type: application/x-compressed → application/x-compressed-tar
Hardware: PC → All
Target Milestone: --- → Bugzilla 3.2
I love this skin, it's such a step forward in look!! From earlier comments, not sure why the inclusion into main trunk has been postponed to 3.2. Is there something missing?
The skin really is nice. Is there some reason this doesn't have a review request on it? I'd be happy to ship this as the default skin.
(In reply to comment #23) > The skin really is nice. Is there some reason this doesn't have a review > request on it? I'd be happy to ship this as the default skin. I am sure it will make a real difference in Bugzilla's appeal. End-users are much more sensitive to look&feel on their very first impression than to features, so "selling" them Bugzilla right now requires a lot of work on making it look sexy. With this skin, the default look would already be much more attractive than the current one. So 100% for it ;-).
Attachment #240550 - Flags: review?(myk)
Could someone provide some links contrasting a landfill installation without this skin to a landfill installation with this skin for the major pages that the skin touches?
Comment on attachment 240550 [details] Mike's Skin (In reply to comment #26) > https://landfill.bugzilla.org/bugzilla-tip/show_bug.cgi?id=1 > http://landfill.bugzilla.org/alternate_skin/show_bug.cgi?id=1 Hmm, indeed, there's lots to recommend this skin, like nice use of borders and colors to distinguish comments. There are also a few changes that I would consider usability regressions, like putting "reply" and "private" controls on a separate line so that every comment header takes two lines instead of one. However, the pros outweigh the cons, and it's probably worth using this skin if we want to use one with such an explicit style (as opposed to providing a more generic style with less use of color), which I'd say we probably should. I've gone through the rules in the CSS files, and they seem ok, but my knowledge of them is stale, so it would be better for someone with more recent knowledge to do this review. Nevertheless, for what it's worth, r=myk.
Attachment #240550 - Flags: review?(myk) → review+
> However, the pros outweigh the cons, and it's probably worth using this skin > if we want to use one with such an explicit style (as opposed to providing a > more generic style with less use of color), which I'd say we probably should. Note: but Bugzilla principals like mkanat, lpsolit, and justdave should make this decision. > I've gone through the rules in the CSS files, and they seem ok, but my > knowledge of them is stale, so it would be better for someone with more recent > knowledge to do this review. > > Nevertheless, for what it's worth, r=myk. Note: but someone besides me should also review this before it goes in.
(In reply to comment #27) > consider usability regressions, like putting "reply" and "private" controls on > a separate line so that every comment header takes two lines instead of one. I also noticed that, and I don't want it either. Also, comment 0 has its header right-aligned which looks broken compared to other, left-aligned, comment headers. Another thing I noticed is that if your screen is quite small, or you have a wide "Details" section at the top (while being logged in), the CC list and flags may be partially out of the light-grey area, which is not very nice. A good example is http://landfill.bugzilla.org/alternate_skin/show_bug.cgi?id=1 See also http://landfill.bugzilla.org/alternate_skin/showdependencygraph.cgi?id=1 Everything else looks great. And this patch will probably forces us to remove the remaining hardcoded CSS styles within HTML templates. Be aware that taking it in the main code will force us to keep 2 skins up-to-date. By the way, is this set of CSS still up-to-date with current 3.1 UI? I would like at least the first point being addressed, about comment headers, before approving this patch for checkin. The second point about data outside the light-grey area may be fixed in a separate bug if it requires too much work. You have 3-4 days left to get it into 3.1.1, else it will be postponed till 3.1.2.
It's been a while since Mike said something here, so I assume he won't be able to work a lot on this. Neither can I. How about we agree on a certain (small) set of well-defined pre-check-in changes to the skin (so that I can do them), then check in, and have the remaining work distributed among several bugs, and thus, possibly several people?
Fixing the comment header was a trivial one-liner. In skins/contrib/Mike/global.css, all you have to do is to remove |display: block;| from .bz_comment_head i { display: block; font-style: normal; } If wurblzap, glob or anyone else agrees with this change, I agree to take it for 3.1.1. But before committing it, we have to choose another name than "Mike" for this new skin. Suggestions?
Flags: approval?
Current skin is called "classic". Calling the new one "modern" could kind of make it easy for people to grasp the before/after effect, couldn't it? Not very original though.... just my 2 pence.
Suggestions are currently posted on my blog: http://lpsolit.wordpress.com/2007/08/08/new-skin-new-name-but-which-name/ It seems it gets more attention than this bug.
Comment on attachment 240550 [details] Mike's Skin I didn't review the patch itself, except to fix the comment header thing. But the UI looks good. r=LpSolit with the one-liner mentioned above addressed on checkin.
Attachment #240550 - Flags: review+
I do not want to maintain two skins. I want to replace the standard skin with this one, and if people don't like it, they are free maintain and ship the old standard skin. We have basically *zero* people who maintain UI right now, adding another skin will make the situation even worse. I don't want to have to test all patches twice. I'm sure that QA doesn't want to have to check the look of every UI twice. So, I just want this to be the default skin. In order to make that happen, this has to be implemented as a patch against the current skin. That will also help us see what's been changed and what's missing. Sorry I didn't comment earlier, I didn't realize I wasn't CC'ed on this bug.
Bug 321556 comment 0 states a very good reason to have a second skin. With a second skin, we can make ourselves aware of skin-programmers woes, and stay aware. This means some overhead, true. If we're any good in just keeping skinning ready at the back of our minds while writing/modifying templates, this overhead may shrink to flipping skins every now and then. Plus, we get the chance to have our code improve in terms of HTML/CSS separation and skinnability. I do think we should go ahead and add a second skin in contrib.
(In reply to comment #36) > I do think we should go ahead and add a second skin in contrib. Yes, I agree. It's not a too big deal to maintain 2 skins anyway as we don't alter the CSS that often. For instance, changing strings doesn't need to be checked against 2 skins.
Attached patch LpSolit's changes, v1 (obsolete) — Splinter Review
Looking at the new skin again, may I suggest these changes? The reasons for these changes are: - We all love rounded corner. Several people mentioned it. - I personally don't like links being underlined in the footer. The selection class comes back, but is darker. - Links in the header and footer are white for better contrast. - Links in the main body are darker and bluer for the same reason (better contrast). At least 2 persons reported the contrast being fainter than in the "Classic" skin, and I agree about that. - Comment header now fits on a single line as already reported by myk and myself. - In the footer, labels and links now have the same size, and are 1pt smaller than the rest of the page to give the focus to the main page. - I use a lovely (?) brown color for the login name to increase the contrast with links. - I add rules for printing which were missing in the new skin. - Unless I miss something, the class name in buglist.css was wrong. If Byron agrees with these changes, could you update alternate_skin/ on landfill?
Attachment #159060 - Attachment is obsolete: true
Attachment #159675 - Attachment is obsolete: true
Attachment #162192 - Attachment is obsolete: true
Attachment #162198 - Attachment is obsolete: true
Attachment #276011 - Flags: review?(wurblzap)
Attachment #276011 - Flags: review?(bugzilla)
Let's summarize what has been said on IRC so far: justdave and myk think we should commit the new skin as an alternate one: justdave: "I think it should be provided in addition to. more choice is always good. I think the idea of forcing us to make sure it remains skinnable is good. I'll abstain from making a call on whether the new skin is worthy of replacing the original or not." myk: "I think it should start out as an alternate skin not enabled by default; then, if folks continue to work on it, and we conclude that it's better than the current one, we should make it the default skin; then, if folks stop maintaining the other skin, we should drop it." kiko thinks it should replace the current one: "to be honest, I don't see that skin being enough of an improvement to run it in parallel. though if you want to /replace/ the existing skin I'd consider it." mkanat already commented in comment 35, and wurblzap in comment 36.
About this skin's name, it looks like "Dusk" is the winner.
lpsolit said: > - We all love rounded corner. Several people mentioned it. For the record. I dislike rounded corners. I like sharp edges. (This is not a reason to keep sharp corners, just a correction of the assertion that everyone loves rounded corners.) justdave: > more choice is always good. I disagree. More choice is often bad if meaningless or unnecessary, and the tyranny of choice can make people miserable. > I'll abstain from making a call on whether the new skin is worthy of replacing > the original or not." Unfortunately that's not possible, since there's no UI module owner. You either need to make that decision yourself or delegate it to someone else.
(In reply to comment #41) > > more choice is always good. > > I disagree. More choice is often bad if meaningless or unnecessary, and the > tyranny of choice can make people miserable. Depends on the context. There are many places where I agree with your assessment. Skins are an area where people traditionally enjoy a little bit of choice however. > > I'll abstain from making a call on whether the new skin is worthy of > > replacing the original or not." > > Unfortunately that's not possible, since there's no UI module owner. You > either need to make that decision yourself or delegate it to someone else. Well, we have three other folks besides me who already have enough authority to make such a decision, and if one of them does, I'll be happy to back it up, or go ahead and make such a choice myself if they decide they can't agree with each other. I figured that was understood, and that was my way of saying I'd rather one of them make the choice. (Mainly because what I like in aesthetics typically doesn't line up with popular opinion) There's been a fair bit more discussion since I said that however, and I'll put my personal vote on having it separate at this point. It's enough of a departure from the existing skin (and there's always people that like the status quo, no matter how bad it is) that we should still offer the choice, at least for a while.
(In reply to comment #42) > (In reply to comment #41) > > > more choice is always good. > > > > I disagree. More choice is often bad if meaningless or unnecessary, and the > > tyranny of choice can make people miserable. > > Depends on the context. There are many places where I agree with your > assessment. Skins are an area where people traditionally enjoy a little bit of > choice however. Sure, agreed. I wasn't saying that choice is always bad, only that it isn't always good.
(In reply to comment #42) > Well, we have three other folks besides me who already have enough authority to > make such a decision Alternate skin!
I would strongly suggest that everybody involved in this discussion read the "The Question of Preferences" section of this article: http://ometer.com/free-software-ui.html
(In reply to comment #45) > I would strongly suggest that everybody involved in this discussion read the > "The Question of Preferences" section of this article: So what? Having a second skin is different from the ability to have more than one skin. The feature allowing Bugzilla to handle several skins has already been implemented. That's what the article would be about.
I thought the point of having skins WAS choice. Why else have a preference where users can select a skin if there is only ever one in the list. I have a skin I have been working on myself that I would love to make available at some point. Perhaps we need to have a separate download location for contributed skins, rather than checking them into the trunk?
(In reply to comment #47) > point. Perhaps we need to have a separate download location for contributed > skins, rather than checking them into the trunk? That's the plan, yes. I think we can have a section on the download page on bugzilla.org with links pointing to available skins, as we have for l10n.
(In reply to comment #47) > Perhaps we need to have a separate download location for contributed > skins, rather than checking them into the trunk? Yeah, as LpSolit said, that's the plan, and I'd be thrilled to have them available there. I'm not against choice, I just am against maintaining two separate skins in CVS. At the least, if we do check this in, I want it to be the default skin, and the old skin should be the alternative. Also, I'm much more in favor of "slate" than "dusk".
Comment on attachment 276011 [details] [diff] [review] LpSolit's changes, v1 >+a, a:link { >+ color: #6169c0; not a fan of this colour -- it has poor contrast against the dark gray background. > .bz_comment_head i { >- display: block; > font-style: normal; > } this doesn't fix the alignment of the header of comment #0. looks like bz_comment_head isn't applied to comment #0, but that's a template problem. otherwise the changes look good :) i think we should maintain both skins, with this one being the default.
Attachment #276011 - Flags: review?(bugzilla) → review-
(In reply to comment #50) > i think we should maintain both skins, with this one being the default. Okay, I'd agree with that.
At this point, we have reached the decision that we should maintain two skins. The reasons in favour are stated in comment 36, and there is a quorum that these give more benefit than the reasons against hurt (comment 35). I think what's not defined yet is whether the new skin should be the default. Personally, I don't like this very much (I think it pretty much missed the major release boat), but there seems to be strong support to do it. So here's my suggestion: we make it the default for new installations, and we don't push it as the default on existing installations. When we have reached a decision on this as well, I'll go through Frédéric's and Byron's proposed changeset and upload a real patch to review. We should preferably not do too many appearance tweaks here seeing that it's not ready for prime time anyway (think the green and light purple colours on the parameters page). These can be done in separate bugs (before 3.2, preferably; otherwise we'd need to relnote the skin as experimental).
I'd like it to be the default for all installations. It's easy enough for the admin to change back the default in the Default Preferences screen. I'd be OK with adding it as a non-default skin at first, as long as it became the default by the release of 3.2.
(In reply to comment #52) > and Byron's proposed changeset and upload a real patch to review. We should ... and apply it to landfill/alternate_skin... (In reply to comment #53) > I'd be OK with adding it as a non-default skin at first, as long as it became > the default by the release of 3.2. That's basically saying that we shall review this decision before releasing 3.2, which I can agree with.
(In reply to comment #50) > not a fan of this colour -- it has poor contrast against the dark gray > background. Can you point me to a place where dark gray is used as the background? And which color would you suggest? > this doesn't fix the alignment of the header of comment #0. Heh, right! I completely forgot this guy. :) > i think we should maintain both skins, with this one being the default. I would tend to agree. That's the best way to maintain it, we would detect imperfections much faster, and admins/users would discover this new skin in 3.2 by default, and if they don't like it they can easily go to their user prefs to select the old one. If we do it the other way, most users will never know a new skin is available.
This patch fixes the problem about the description header and is independent of the new skin. It has to land first. Marc, if you are faster than glob, feel free to review it.
Attachment #276120 - Flags: review?(bugzilla)
Attachment #276011 - Flags: review?(wurblzap)
Flags: blocking3.1.1+
Personally, I'd like to see the alternative skin not becoming the default one. But that's just my personal preference from a user point of view because I like the colors of the default scheme better. IMO the alternative skin just gives a different color scheme and does not solve any usability issues. I'd rather like to see more effort going into solve those. Is there a meta bug that tracks usability issues?
> which color would you suggest? a #454972 visited #901B1B
Comment on attachment 276120 [details] [diff] [review] Fix comment 0 (aka description) header, v1 yup, this fixes the problem r=glob
Attachment #276120 - Flags: review?(bugzilla) → review+
Attached patch LpSolit's changes, v2 (obsolete) — Splinter Review
Attachment #276011 - Attachment is obsolete: true
Attachment #276487 - Flags: review?(bugzilla)
Comment on attachment 276487 [details] [diff] [review] LpSolit's changes, v2 r=glob
Attachment #276487 - Flags: review?(bugzilla) → review+
I checked in my patch about comments.html.tmpl (attachment 276120 [details] [diff] [review]). Marc, all you have to do is to check in the new dir in skins/contrib/ with my changes suggested in "LpSolit's changes, v2". Checking in skins/standard/global.css; /cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v <-- global.css new revision: 1.39; previous revision: 1.38 done Checking in template/en/default/bug/comments.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/comments.html.tmpl,v <-- comments.html.tmpl new revision: 1.32; previous revision: 1.31 done
Flags: approval? → approval+
Keywords: relnote
Attached patch PatchSplinter Review
Full patch, including attachment 276487 [details] [diff] [review] and additional tweaks. Carrying forward Byron's r+.
Attachment #240550 - Attachment is obsolete: true
Attachment #276487 - Attachment is obsolete: true
Attachment #276684 - Flags: review+
Comment on attachment 276684 [details] [diff] [review] Patch Frédéric, can you please sign off on the additional tweaks, as mentioned on IRC?
Attachment #276684 - Flags: review?(LpSolit)
Comment on attachment 276684 [details] [diff] [review] Patch I didn't apply the patch, but it looks good. Marc says only 2 CSS files are part of Dusk/ because the other ones are left unchanged. r=LpSolit
Attachment #276684 - Flags: review?(LpSolit) → review+
Checked in with an additional line modified as agreed with Frédéric on IRC: in Bugzilla/Install.pm, the default skin for new installations (and installations upgrading from a pre-skin-enabled version) is set to Dusk. Checking in Bugzilla/Install.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install.pm,v <-- Install.pm new revision: 1.15; previous revision: 1.14 done Checking in Bugzilla/User/Setting/Skin.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/User/Setting/Skin.pm,v <-- Skin.pm new revision: 1.4; previous revision: 1.3 done Checking in skins/.cvsignore; /cvsroot/mozilla/webtools/bugzilla/skins/.cvsignore,v <-- .cvsignore new revision: 1.2; previous revision: 1.1 done RCS file: /cvsroot/mozilla/webtools/bugzilla/skins/contrib/Dusk/buglist.css,v done Checking in skins/contrib/Dusk/buglist.css; /cvsroot/mozilla/webtools/bugzilla/skins/contrib/Dusk/buglist.css,v <-- buglist.css initial revision: 1.1 done RCS file: /cvsroot/mozilla/webtools/bugzilla/skins/contrib/Dusk/global.css,v done Checking in skins/contrib/Dusk/global.css; /cvsroot/mozilla/webtools/bugzilla/skins/contrib/Dusk/global.css,v <-- global.css initial revision: 1.1 done Checking in template/en/default/bug/navigate.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/navigate.html.tmpl,v <-- navigate.html.tmpl new revision: 1.9; previous revision: 1.8 done Checking in template/en/default/bug/show-multiple.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show-multiple.html.tmpl,v <-- show-multiple.html.tmpl new revision: 1.36; previous revision: 1.35 done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago17 years ago
Resolution: --- → FIXED
The relnote needs to point out that a previously checksetup.pl-generated skins/contrib/ dir needs to be removed before a CVS update.
Whiteboard: relnote comment 67
Added to the release notes for Bugzilla 3.2 in a patch on bug 432331.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: