Closed
Bug 148679
Opened 22 years ago
Closed 22 years ago
Permit multiple stylesheets in header template
Categories
(Bugzilla :: User Interface, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: bugreport, Assigned: myk)
Details
Attachments
(1 file, 3 obsolete files)
1.73 KB,
patch
|
jouni
:
review+
jouni
:
review+
|
Details | Diff | Splinter Review |
Bugzilla 2.16rc1 templates assume that there will be a maximum of one style_url for a page. Change this to a list of style_urls. (One cool thing, a string seems to behave as a list of one item, so it is backward-compatible with any templates that don;t know this can be a list)
Reporter | ||
Comment 1•22 years ago
|
||
This patch permits the calling template to pass a list of stylesheets instead of a single stylesheet to header.html.tmpl. All other things being equal, the LAST on the list has precedence.
Comment 2•22 years ago
|
||
Comment on attachment 85986 [details] [diff] [review] Change to header.html.tmpl to permit multiple stylesheets Is the fact that a scalar is a one-element list something we can count on, or is it just an artifact of how stuff is stored internally by TT?
Reporter | ||
Comment 3•22 years ago
|
||
You're probably right. I reread all the template documentation and the ability of FOREACH to iterate once if it has a string instead of a list doesn't seem to be guaranteed anywhere. As far as I can tell, the only place that would have to be updated to have an extra set of square brackets is list/list.html.tmpl It probably pays to make the change there as well. diff -u -r1.4 list.html.tmpl --- list.html.tmpl 21 May 2002 17:53:40 -0000 1.4 +++ list.html.tmpl 3 Jun 2002 03:02:52 -0000 @@ -24,7 +24,7 @@ [%############################################################################% ] [% DEFAULT title = "Bug List" %] -[% style_url = "css/buglist.css" %] +[% style_url = [ "css/buglist.css" ] %] [% qorder = order FILTER url_quote IF order %]
Comment 4•22 years ago
|
||
Comment on attachment 85986 [details] [diff] [review] Change to header.html.tmpl to permit multiple stylesheets >Index: template/en/default/global/header.html.tmpl >=================================================================== >+ # style_url: list. list of CSS styles. This parameter should be "style_urls" if we're going to change list.html.tmpl anyway; otherwise the added [ ] around a single url won't make much sense to a casual reader. As for that comment quoted above: how about "List of URLs to CSS stylesheets." The current wording can be understood as a list of actual CSS rules, especially when separated from the context. >+ [% FOREACH this_style_url = style_url %] ... and then you could write this as FOREACH style_url = style_urls, which seems to be some sort of a convention here these days.
Reporter | ||
Comment 5•22 years ago
|
||
I certainly could make this style_urls and make the change in list.html as well, but that would break compatibility for anyone who had used stylesheets in customizations. Before I change style_url to style_urls in the interface, does anyone have a strong opinion?
Comment 6•22 years ago
|
||
If its a list, it should be plural. I'm curious why you'd need this, though. Where would this be used?
Reporter | ||
Comment 7•22 years ago
|
||
The origin of this is my attempts to color-code the comments. When generating the comments in the edit template, I wanted to code both the font and the background of that section. When using the same routines to generate the comments formatted for printing, I want to code the font differrently and not apply that the the background color. Ordinarily, I would have just used 2 differrent stylesheets. However, that would mean that the first time I decide to use a stylesheet, I am precluding anyone else from ever using a differrent one. Gerv and I exchanged some email and agreed that I should create a, hopefully very localized, patch. A good case in point for this is someone who wants to enlarge all the fonts and puts P,TR,BODY { font-size : 150% } in a stylesheet. If differrent Bugzilla components use differrent stylesheets, then this would have to be inserted into evry stylesheet. By permitting multiple stylesheets, however, there can be a local stylesheet listed in addition to the stylesheet designed by the template authout.
Comment 8•22 years ago
|
||
> I certainly could make this style_urls and make the change in list.html as well,
> but that would break compatibility for anyone who had used stylesheets in
> customizations.
This is why we are doing it now, rather than later :-) Template interfaces are
still fluid, as we haven't released.
Gerv
Reporter | ||
Comment 9•22 years ago
|
||
OK... style_urls it is
Attachment #85986 -
Attachment is obsolete: true
Comment 10•22 years ago
|
||
Comment on attachment 86215 [details] [diff] [review] Multiple stylesheet patch using "style_urls" >+ # style_urls: list. list of CSS styles. "list of URLs to CSS style sheets." Other than that, r=gerv. Gerv
Attachment #86215 -
Flags: review+
Comment 11•22 years ago
|
||
Comment on attachment 86215 [details] [diff] [review] Multiple stylesheet patch using "style_urls" Umm yeah, I agree with Gerv. Make a new patch with the mentioned comment fix and you have r=jouni on it.
Reporter | ||
Comment 12•22 years ago
|
||
Attachment #86215 -
Attachment is obsolete: true
Comment 13•22 years ago
|
||
Comment on attachment 86220 [details] [diff] [review] Revision to 86215 with comment change from review >+ # style_urls: list. List URLs to CSS style sheets. Nit: Make that 'List of URLs to CSS style sheets.' -- :-)
Comment 15•22 years ago
|
||
Comment on attachment 86221 [details] [diff] [review] Patch with clearer comment - ficntionally identical to 86220 Ok then :-) r=jouni, and there was a r=gerv two attachments up, so I'll mark that as well. Ready for checkin.
Attachment #86221 -
Flags: review+
Comment 16•22 years ago
|
||
Is this meant for the 2.16 branch? Gerv
Comment 17•22 years ago
|
||
Gerv: Per bug 69654 comment 27 and comment 8 on this bug, I'd say that this should be in 2.16. Reconsider yourself, though; both the mentioned sources are written by you :-) I'd do it now because this involves an interface change. Marking 2.16, bump if you change your mind.
Target Milestone: --- → Bugzilla 2.16
Comment 18•22 years ago
|
||
As long as the patch is available and works there, let's go ahead and put this in 2.16. It won't hurt anything and it's already here.
Comment 19•22 years ago
|
||
checking in on behalf of Joel since he doesn't have checkin privs to my knowledge... HEAD: Checking in template/en/default/global/header.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v <-- header.html.tmpl new revision: 1.10; previous revision: 1.9 done Checking in template/en/default/list/list.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v <-- list.html.tmpl new revision: 1.5; previous revision: 1.4 done BUGZILLA-2_16-BRANCH: Checking in template/en/default/global/header.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v <-- header.html.tmpl new revision: 1.5.2.5; previous revision: 1.5.2.4 done Checking in template/en/default/list/list.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v <-- list.html.tmpl new revision: 1.3.2.2; previous revision: 1.3.2.1 done
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•