Open Bug 385927 Opened 17 years ago Updated 2 years ago

Make expander useful

Categories

(Toolkit :: UI Widgets, defect)

Product:
Toolkit
Component:
UI Widgets
Type:
defect

Tracking

()

People

(Reporter: mossop, Unassigned)

References

Details

Attachments

(6 files, 1 obsolete file)

Mac example
7.39 KB, image/png
Details
Linux example
5.94 KB, image/png
Details
Windows example
4.60 KB, image/png
Details
v1.1
12.15 KB, patch
enndeakin
: review-
Details | Diff | Splinter Review
Expander widget in gtk
4.40 KB, image/png
Details
contains rules to equalize the look and feel across platforms and gecko versions, among other things
1.44 KB, text/css
Details 
Going to do some improvements to the expander element mking it more useful. In particular the following:

* Remove the clear button.
* Add some events when the content is shown/hidden.
* Fix the theming on the main platforms.
I think there were some accessibility issues with it too. That was long ago, so I don't remember the details though.
Attached image Mac example — 

    
    

    
  

      
      
      
      

        Attached image
          
        Linux example
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached image
          
        Windows example
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch rev 1 (obsolete)
        — 
          — Splinter Review
      

    


  
This is practically a rewrite of the expander widget. It provides a twisty and label caption that when clicked open out to display the children of the widget.

It provides an open attribute to set the initial state of the control and a disabled attribute to disable the control's behaviour.

Additionally when the children are shown/hidden a show/hide event is dispatched to the expander widget and to any onshow and onhide attributes.

Screenshots have been provided for the main platforms.

        Attachment #271206 -
        Flags: review?(enndeakin)

    
    

    
  
Comment on attachment 271206 [details] [diff] [review]
patch rev 1

Index: toolkit/content/widgets/expander.xml
 
   <binding id="expander" display="xul:vbox">

Remove the display part as well.

+        <xul:button type="disclosure" class="expanderButton" 
+                    anonid="disclosure" xbl:inherits="open,disabled"/>

Maybe tabindex should be inherited too, although I can't recall if that works properly.

+      <method name="_fireEvent">
+        <parameter name="aType"/>
+        <body>
+          var event = document.createEvent("Events");
+          event.initEvent(aType, true, false);
+          
+          // handle dom event handlers
+          this.dispatchEvent(event);
+          

This should check the the return value to see if it was cancelled. At least, that's what the other similar calls like this do.

+          // handle any xml attribute event handlers
+          var handler = this.getAttribute("on"+aType);
+          if (handler != "") {
+            var fn = new Function("event", handler);
+            fn.apply(this, [event]);

Same here.

+      <property name="disabled">
+        <setter>
+          if (this.disabled == val)
+            return;

This shouldn't be necessary. Attribute modifications already check for equivalence.

+          this._content.collapsed = !val;
+          this.setAttribute("open", val ? "true" : "false");

Would this be easier with a style rule like:

.whatever:not(open[true]) {
  visibility: collapse;
}

The constructor could be removed then too.

+
+          this._fireEvent(val ? "show" : "hide");

Not sure if I like having two events for this. Ideally, this should just be a command event.

Also, add a label property.

You should be able to just copy the label/disabled/tabindex properties from general.xml

I have a feeling though that <expander> should just be the arrow button, and a separate element <expandbox> should be the box with an expander and collasable box. That would allow expander to just inherit from general.xml#basecontrol. Not sure though.

        Attachment #271206 -
        Flags: review?(enndeakin) → review-

    
  
No longer blocks: 297903

    
    

    
  

      
      
      
      

        Attached patch
          
          
        v1.1
        — 
          — Splinter Review
      

    


  
Addresses comments, except for the last one since you weren't sure.
Assignee: dtownsend → sdwilsh

        Attachment #271206 -
        Attachment is obsolete: true
Status: NEW → ASSIGNED

        Attachment #277052 -
        Flags: review?(enndeakin)

    
    

    
  
Comment on attachment 277052 [details] [diff] [review]
v1.1

>+
>+.contentContainer:not(open["true"]) {
>+  visibility: collapse;
>+}

This rule has a syntax error, and doesn't actually cause the expander to show/hide the element.

Also, the rule should really be in xul.css rather than in the themes.

        Attachment #277052 -
        Flags: review?(enndeakin) → review-
Target Milestone: --- → mozilla1.9 M8
Target Milestone: mozilla1.9 M8 → mozilla1.9 M9

    
    

    
  
I don't have time for this - if someone else wants to pick it up, please feel free to do so.
Assignee: comrade693+bmo → nobody
Status: ASSIGNED → NEW
Target Milestone: mozilla1.9 M9 → ---

    
    

    
  
In fx3b3 the hbox containing the button and caption doesn't open/close the expander onclick anymore and doesn't change the cursor to a hand (hover).

It used to do that up to fx3b2. I think having to click the tiny button is rather inconvenient. Unfortunately XBL confuses the heck out of me, so I don't see myself fixing this.

Or was this an intentional change? I hope not. I'm using expander in an extension.

    
    

    
  
(In reply to comment #10)
> In fx3b3 the hbox containing the button and caption doesn't open/close the
> expander onclick anymore and doesn't change the cursor to a hand (hover).
> 
> It used to do that up to fx3b2. I think having to click the tiny button is
> rather inconvenient. Unfortunately XBL confuses the heck out of me, so I don't
> see myself fixing this.
> 
> Or was this an intentional change? I hope not. I'm using expander in an
> extension.
> 

The expander widget hasn't changed in nearly a year, and it is not likely to change in the near future either.


    
    

    
  

      
      
      
      

        Attached image
          
        Expander widget in gtk
        — 
      

    


  
The expander widget in e.g. the page information dialog, doesn't look like its native sibling in gtk.

    
    

    
  
We do want this for 1.9.2, we're starting to do more with progressive disclosure widgets, and I'd rather these be useful and easy to use than having to fake it.
Flags: wanted1.9.2+

    
    

    
  


  
You might (possibly) be interested in CSS changes I made to the expander for the extension 'SMS Sidebar' in order to make it usable as a ui widget. Most of the rules in the attachment are about unifying the look of the expander across platforms and gecko versions while preserving (or adding) the correct twisty for the platform. A few rules are specific to my extension, I don't have time right now to sort it all out...
Maybe it's interesting to see what was necessary to 'make it usable' without touching the Fx styles. (It was quite excruciating...)

If you want to test the expanders you can get the extension here:
https://addons.mozilla.org/en-US/firefox/addon/6617

I'd love to work on this bug, especially since I also authored an 'Expandable Widget' for jQuery. But it's unlikely I'll find the time. :(

    
  
Severity: normal → S3

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: