Closed
Bug 262592
Opened 20 years ago
Closed 19 years ago
Enable templates to use a cookies to remember UI preferences and hide/expose content
Categories
(Bugzilla :: User Interface, enhancement, P3)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.22
People
(Reporter: bugreport, Assigned: u197037)
Details
Attachments
(3 files, 5 obsolete files)
2.62 KB,
patch
|
Details | Diff | Splinter Review | |
2.72 KB,
patch
|
Details | Diff | Splinter Review | |
5.53 KB,
patch
|
bugreport
:
review+
|
Details | Diff | Splinter Review |
Site customizations often need to make template customizations to simplify parts of the user interface. Since some users want an extremely simple interface while others want the full "power-user" interface, templates should be able to control a cookie to remember UI preferences and make presentation decisions baed on those preferences. This logic needs to be available to the templates without adding it to every script. One of the standard CGIs (CGI.pm or Template.pm) should a) check for the existance of a SetBugzillaPrefs form variable and, if found, create/update the Bugzilla_Prefs cookie. b) if the Bugzilla_Prefs cookie is found, pass it to the templates in the form of a hash. So, the cookie could be Bugzilla_Prefs: bugentry,advanced,warnsecure,yes And a template could make decisions using test like [% IF prefs.bugentry == 'advanced' %] And, when a form variable SetBugzillaPrefs is processed with a value... bugentry,simple,alwaysccme,yes the cookie would subsequently become... Bugzilla_Prefs: bugentry,simple,warnsecure,yes,alwaysccme,yes
Reporter | ||
Comment 1•20 years ago
|
||
OK, this can be done more easily.... Set the cookies with javascript and read them by.... justdave [% USE Bugzilla %] joel like pass the CGI object to the templates justdave [% cgi = Bugzilla.cgi %] justdave [% mypref = cgi.cookie("mycookiename") %] joel I didn't know we could do a USE in a template justdave we can for the Bugzilla object justdave that's what Bugzilla/Template/Plugin/Bugzilla is for :) justdave USE in a template loads a template plugin justdave we have a plugin that supplies access to the Bugzilla object
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → WONTFIX
Reporter | ||
Comment 2•20 years ago
|
||
Actually, I will both rename and reopen this.... The new plan is to have a js/ file that can be easily included in templates to help them manage a cookie and hide/expose content. The initial application of this is to facilitate a site customization that presents a stripped down entry forms to new users, but lets more advanced users use the full form. So, this will be a javascript library with functions to 1) hide/reveal document elements of a specified class 2) update cookies from buttons... initialize button appearance to indicate current state of cookies. 3) onload, check cookies and call hide/reveal code The code should be able to live in js/* with no code calling it. It should be very easy to a) plant code in the user_prefs templates to create a bunch of buttons to set/reset these cookies. b) plant code in any Bugzilla template to use the cookies onload to hide/reveal form elements of a particular class.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Enable templates to use a Bugzilla_Prefs cookie to remember UI preferences → Enable templates to use a cookies to remember UI preferences and hide/expose content
Reporter | ||
Comment 3•20 years ago
|
||
This is a hacked up create template that has Simple and Advanced buttons, maintain the prior state via cookies, and hides/reveals the dependencies rows (indicated by the "advanced" class) based on the state of simple versus advanced. This needs to be factored out and modularized, but I think it shows the concept.
Reporter | ||
Updated•19 years ago
|
Status: REOPENED → ASSIGNED
Reporter | ||
Updated•19 years ago
|
Assignee: bugreport → dennis.melentyev
Status: ASSIGNED → NEW
Reporter | ||
Updated•19 years ago
|
Priority: -- → P3
Target Milestone: --- → Bugzilla 2.22
Status: NEW → ASSIGNED
Priority: P3 → --
Target Milestone: Bugzilla 2.22 → ---
Reporter | ||
Comment 4•19 years ago
|
||
Just to clarify the intent in comment 3, ideally this would work as follows... 1) In a site with no need to use these cookies, no template customization would invoke any of the new code, so it would not affect the site. 2) Adding code to templates to either hide/reveal sections based on the class or to would involve adding only a very simple block like [% PROCESS cookiehide cookie = "advanced" class = "advancedclass" %] and buttons like <input type="button" value="advanced" onclick='return reveal("advanced")'> <input type="button" value="simple" onclick='return hide("advanced")'> [Warning - my example here may be very incorrect, but is intended to convey the general intent]
Yes, I think the same way: adding particular class name as an argument to hide/reveal function will let template developer to add as many hidable sections on the page as wanted. With separate buttons to hide/reveal each section. Also this would need tunable cookie sets. In order to do this, the hide/reveal "API" should receive not only the class name, but some cookie suffix. So, cookie name could end up in something like: BugzillaUI_<suffix>_<class>=hide (or =show, or =visible, or =reveal...) This allow developer to create both common cross page cookies and the per-page ones. PS. I get back the Priority=P3 and Milestone=2.22 values set by you, Joel
Priority: -- → P3
Target Milestone: --- → Bugzilla 2.22
Minimalistic test.cgi and template for it. Provides test capabilities
Reporter | ||
Comment 8•19 years ago
|
||
Comment on attachment 181526 [details] [diff] [review] new files to provide hide/reveal/collapse functionality Initial comments by inspection...: 1) We should use the cookiepath parameter to distinguish between multiple sites on the same host. It is available to template code as Param("cookiepath") and will be blank on many systems. 2) Since the function being called in TUIButton is already known to be a "button," it does not need to be called "btnname." "name" would be much more intuitive. 3) We really need to figure out where these templates should go. global/TUI* does not sound right, but I don't have a clear idea at the moment. 4) If the cookie is not previously defined, could we tell TUIProcess what we want to use for the default behavior? If we do this, we don't just want to act on the absence of the cookie because some browser configurations will not let us set cookies from javascript. Rather, if the cookie is absent, we want to set it to a specified value and then look at it. The example for this would be a UI change that hides some elements (by default) unless a user enables an advanced mode. If we cannot control cookies, we want the elements to remain visible. If the user has never done anything to set the cookies, we want to presume that the user does not want to see the elements. I'll experiment with this later.
Reporter | ||
Comment 9•19 years ago
|
||
Ignore my previous comment about cookiepath. It looks like javascript takes care of this for you. ----------- I tried validating the test page... HTML 4.01 Transiitonal does not like <HR /> and <INPUT whatever /> ----------- Actually, we should move the javascript functions themselves into a file in the js/ directory and let the calling template (like invoke them with... <script src="js/TUI.js" type="text/javascript"></script> This will let the browser cache the scripts themselves. followed by, for each cookie... TUI_tweak('advanced', 'hide') The second argument is the name of the cookie, and the 2nd argument to TUI_tweak sets the default, then you can move the CASE statement that is currently in TUIButton into the javascript itself and then each button can be emitted using... <input type="button" value="Advanced" onclick="return TUI_change('advanced', 'reveal')"> <input type="button" value="Simple" onclick="return TUI_change('advanced', 'hide')"> ------------------- If I look at what it would take to hide/reveal a bunch of content on the bug creation form, I would change only template/en/default/bug/create/create.html.tmpl to... 1) Add js/TUI.js to javascript_urls when create.html.tmpl calls global/header.html.tmpl 2) Change some of the form elements to add the 'advanced' class to them 3) Add the call to TUI_Tweak shown above 4) Add the buttons I tried this out with.... <table class="baa"> <tr><td>one</td></tr> <tr class="foo simple"><td>two</td></tr> <tr><td>three</td></tr> And everything seems to work as well. ------ When you do the next revision of this, I suggest using a patch against the bug creation form as the demo. It will show exactly how little needs to be changed without requiring you to build a whole new cgi.
Comment 10•19 years ago
|
||
(In reply to comment #5) > So, cookie name could end up in something like: > BugzillaUI_<suffix>_<class>=hide (or =show, or =visible, or =reveal...) > This allow developer to create both common cross page cookies and the per-page > ones. A thought: to avoid cookie inflation -- can we put the cross-page cookies into one, hash-style? Perhaps the per-page ones as well, per page...
Assignee | ||
Comment 11•19 years ago
|
||
(In reply to comment #9) > ----------- > I tried validating the test page... HTML 4.01 Transiitonal does not like <HR /> > and <INPUT whatever /> > ----------- changed this to be just <input ...></input> > Actually, we should move the javascript functions themselves into a file in the > js/ directory and let the calling template (like invoke them with... > > <script src="js/TUI.js" type="text/javascript"></script> > > This will let the browser cache the scripts themselves. Does the following looks reasonable? .... set defaults, without setting cookies [% TUISuffices = [ [ 'advanced', 'collapse'], ['simple', 'show'] ] %] let header.html.tmpl register our TUI.js (could be hidden into TUIInit.html.tmpl) [% javascript_urls = [ 'TUI.js' ] %] [% PROCESS header.html.tmpl %] .... Add some buttons [% PROCESS TUIButton.html.tmpl ....%] [% PROCESS TUIButton.html.tmpl ....%] .... Initially process all tags, giving defaults from TUISuffices list of pairs if cookies are not set/enabled. [% PROCESS TUIProcess.html.tmpl %] .... > When you do the next revision of this, I suggest using a patch against the bug > creation form as the demo. It will show exactly how little needs to be changed > without requiring you to build a whole new cgi. Ok.
Assignee | ||
Comment 12•19 years ago
|
||
(In reply to comment #10) > (In reply to comment #5) > > So, cookie name could end up in something like: > > BugzillaUI_<suffix>_<class>=hide (or =show, or =visible, or =reveal...) > > This allow developer to create both common cross page cookies and the per-page > > ones. > > A thought: to avoid cookie inflation -- can we put the cross-page cookies into > one, hash-style? Perhaps the per-page ones as well, per page... Could be done on per-suffix base. Something like: "BugzillaUI_<Suffix>=class:default,class:default,class:default,...;"
Reporter | ||
Comment 13•19 years ago
|
||
The proposal in comment 11 looks good. Please do make sure that it is possible for a page to use TUIProcess even if that page does not have any buttons on it. For example, some sites may put the buttons on a seperate page.
Assignee | ||
Comment 14•19 years ago
|
||
(In reply to comment #13) > The proposal in comment 11 looks good. Please do make sure that it is possible > for a page to use TUIProcess even if that page does not have any buttons on it. > For example, some sites may put the buttons on a seperate page. With minor restriction of cookie with the appropriate suffix should be set and page will redraw only after it's reload. Even more, one should be able to put all that buttons into separate "prference" page.
Assignee | ||
Comment 15•19 years ago
|
||
Reworked patches. Including comments both from Joel Peshkin and Marc Schumann
Attachment #181526 -
Attachment is obsolete: true
Assignee | ||
Comment 16•19 years ago
|
||
The test page patch is generated against the create.html.tmpl instead of specialy designed test.cgi
Attachment #181527 -
Attachment is obsolete: true
Comment 17•19 years ago
|
||
Comment on attachment 181881 [details] [diff] [review] The code itself This is quite good. My comments are mostly little minor things. >+++ template/en/default/global/TUIInit.html.tmpl 2005-04-27 03:57:00.000000000 +0000 I was talking to Joel about the naming on these scripts, and I have a nitpicky problem with calling them TUI*, only because a developer not familiar with this part the source will then have to go figure out what TUI means. Could we instead use something like "UI_Toggle" for example? Just something more outwardly descriptive. >+ [% BLOCK TUIjscript %] This block really needs a comment describing its interface. >+ <!-- >+ var TUISuffices = new Array; >+ [% FOREACH suffix = TUISuffices %] nit: s/Suffices/Suffixes/ >+++ template/en/default/global/TUIProcess.html.tmpl 2005-04-27 01:09:49.000000000 +0000 There are several small templates in use. Would it work better if they were in a single file, separated by BLOCKs and called using INCLUDE? >--- /dev/null 2005-04-11 14:18:51.000000000 +0000 >+++ js/TUI.js 2005-04-27 02:55:36.000000000 +0000 >+ function split(source, delimiter) { Could this be accomplished by calling the split method that's built in to the javascript String object? >+ function TUI_tweak( cookie ) { >+ var dc = document.cookie; >+ var begin = -1; >+ var end = -1; >+ var cookiePrefix = "Bugzilla_TUI_"+cookie+"="; >+ >+ // Process cookies, fetch ones with our prefix >+ while(-1 != (begin = dc.indexOf(cookiePrefix, end))) { That while line looks like more complication (in the readability sense) than necessary. Could the variable assignment be moved out of it? That's about all I have so far, aside from some things Joel told me he's already covering.
Reporter | ||
Comment 18•19 years ago
|
||
I think we are almost finished with this. I really would like to get rid of the templates as part of the patch. Including the calls to the functions in the way I show in comment 9 should be just fine and it reduced the potential for clutter by getting this down to one .js file. Please do make that change. I still do need to do some more testing.
Assignee | ||
Comment 19•19 years ago
|
||
As Joel requiested, got rid of templates at all. Encounted Erik's remark on bloated while() statement Changed TUISuffices to TUIClasses, as it's more appropriate name
Attachment #181882 -
Attachment is obsolete: true
Assignee | ||
Comment 20•19 years ago
|
||
As Joel requiested, got rid of templates at all. Encounted Erik's remark on bloated while() statement Changed TUISuffices to TUIClasses, as it's more appropriate name
Attachment #181881 -
Attachment is obsolete: true
Reporter | ||
Comment 21•19 years ago
|
||
Almost .. (very close) The default behavior of this if the cookie has never been set is to hide the advanced content. Just need to fix that.
Assignee | ||
Comment 22•19 years ago
|
||
(In reply to comment #21) > Almost .. (very close) > > The default behavior of this if the cookie has never been set is to hide the > advanced content. Just need to fix that. Hmm.. Works for me. Just removed all cookies for test site in mozilla cookie manager and opened patched enter_bug.cgi: all advanced content is collapsed, simple - shown. As it was expected by setting defaults to "advanced:collapse,simple:show" PS. The same behaviour in "clean" IE (all cookies were cleaned up) PPS. Didn't I get your point right?
Reporter | ||
Comment 23•19 years ago
|
||
I said that completely backwards, but I'll retest first. I need to make sure that the default is used if there had never been a cookie before (and the default could be controlled) AND that the default is NOT enforced if javascript is not permitted to set cookies.
Assignee | ||
Comment 24•19 years ago
|
||
Revised code to handle disabled cookies policy. Now, show everything, if cookies are not supported. But, if cookies was set earlier and are disabled now, code will use those saved cookie values. Despite it would be unable to set the new defaults. Also, code will issue an alert() if user will attempt to use the controlling buttons while cookies are disabled (in both cases).
Attachment #181962 -
Attachment is obsolete: true
Reporter | ||
Comment 25•19 years ago
|
||
Comment on attachment 182161 [details] [diff] [review] The code itself r=joel [The alerts should be removed on checkin -- better for it to gracefully degrade .... also a comment should reference this bug for usage examples -- both can be done on checkin]
Attachment #182161 -
Flags: review+
Reporter | ||
Updated•19 years ago
|
Flags: approval?
Target Milestone: Bugzilla 2.22 → Bugzilla 2.20
Updated•19 years ago
|
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Comment 26•19 years ago
|
||
Do we have any pages that uses this yet?
Assignee | ||
Comment 27•19 years ago
|
||
Not in Bugzilla itself, but in derived works.
Reporter | ||
Comment 28•19 years ago
|
||
This is more in the same category as hooks where none of the standard code uses the mechanism but it provides a standard mechanism to customizers.
Comment 29•19 years ago
|
||
ok, works for me.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago → 19 years ago
Flags: approval? → approval+
Resolution: --- → FIXED
Comment 30•19 years ago
|
||
er, I didn't mean to mark that fixed, not even sure how I did it...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 31•19 years ago
|
||
Should I re-request approval? As for me, it is RESOLVED FIXED, but I'm not sure should I mark it this way myself now, or wait for final approval/cvs checkin?
Status: REOPENED → ASSIGNED
Reporter | ||
Comment 32•19 years ago
|
||
Checking in js/TUI.js; /cvsroot/mozilla/webtools/bugzilla/js/TUI.js,v <-- TUI.js initial revision: 1.1 done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 33•19 years ago
|
||
Thanks!
Updated•11 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
•