Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9086 closed enhancement (fixed)

Allow CSS height on dijit.Menu

Reported by: glathoud Owned by:
Priority: high Milestone: 1.4
Component: Dijit Version: 1.3.0
Keywords: Cc:
Blocked By: Blocking:

Description

The goal is to permit full CSS styling (esp. height and overflow-y) of the menu that appears when clicking on a dojox.form.DropDownWidget.

Use case: this is particularly important when having various sorts of popup menus on a page, where:

  • some menus have too many items and need to have their height constrained (e.g. from a DropDownSelect),
  • other menus need *not* have their height constrained, because it would perturbate the behavior of dijit.place() .

Issue: In dojo 1.3, file dijit/templates/Menu.html , the top-level node is a <table> . This means that its height cannot be fully controlled.

Proposed patch: the top-level node in dijit/templates/Menu.html should be a <div>, with the <table> as child. The top-level <div> can then be freely styled using for example the CSS class dojoxDropDownSelectMenu .

Note: this can also apply to earlier version of Dojo, like 1.1, where dijit.Menu.templateString = '<table>...</table>'

Attachments (2)

popup.diff (623 bytes) - added by glathoud 10 years ago.
Patch for dojo 1.3 r17461 : dijit._base.popup
_HasDropDown.diff (176 bytes) - added by glathoud 10 years ago.
Patch for dojo 1.3 r17461 : dojox.form._HasDropDown

Download all attachments as: .zip

Change History (11)

comment:2 Changed 10 years ago by glathoud

Concrete example of CSS use:

.dojoxDropDownSelectMenu {
  height:300px;
  overflow-y:auto;
}

This works on a <div>, but not on a <table> .

comment:3 Changed 10 years ago by glathoud

...but this seems to work only partially in IE7: the scrollbar appears, but the <table> is still not limited to the desired 300px.

In the end, the only thing that worked in all browsers was:

  • not to patch the template string, which remains a <table>.
  • patch the code of dijit.form.DropDownButton and dijit.popup.open() to add an optional extra CSS class name to the top-level popup DOM node.

comment:4 Changed 10 years ago by glathoud

Here are the patches for dojo 1.1 . I should be able to add the patches for dojo 1.3 later.

dijit/form/Button.js :

115a117,118
>     popupExtraClass : "",   /* String | Array of String */
> 
248c251,252
< 			}
---
> 			},
>                     extraClass: this.popupExtraClass

dijit/_base/popup.js :

58a59,60
>             //              extraClass: String|Array
>             //                      extra CSS classes to add to the top-level popup, in addition to the standard "dijitPopup" class.
80c82,87
< 		wrapper.className="dijitPopup";
---
> 
>             var classNameArray = [ 'dijitPopup' ];
>             if ( typeof args.extraClass === 'string' ) { classNameArray.push( args.extraClass ); }
>             else if ( ( typeof args.extraClass === 'object' ) && args.extraClass.length ) { classNameArray = classNameArray.concat( args.extraClass ); }
> 	    wrapper.className= classNameArray.join( ' ' );
> 

Changed 10 years ago by glathoud

Attachment: popup.diff added

Patch for dojo 1.3 r17461 : dijit._base.popup

Changed 10 years ago by glathoud

Attachment: _HasDropDown.diff added

Patch for dojo 1.3 r17461 : dojox.form._HasDropDown

comment:5 Changed 10 years ago by glathoud

Dojo 1.3 : With the two attached patches (attachment:popup.diff and attachment:_HasDropDown.diff), it is possible to add an extra CSS class to the popup, and therefore fully style each drop-down/popup individually.

comment:6 Changed 10 years ago by bill

Sorry this hasn't been looked at. Those patch files are in a strange format, not generated with the patch program (or the patch menu from various IDE's).

Also, have you filed a CLA? You need to before we can accept patches.

Finally, not sure we need to add an extra <div> to Menu as the popup manager itself creates a wrapper <div>.

comment:7 Changed 10 years ago by Nathan Toone

Resolution: fixed
Status: newclosed

I also believe that this may be addressed with the changes that have recently been made to _HasDropDown. You can now specify the max-height of the dropdown.

Closing for now. Reopen with a test case, if you are still seeing issues.

comment:8 Changed 10 years ago by bill

Milestone: tbd1.4

comment:9 Changed 10 years ago by glathoud

Sorry, for some reason, I missed the activity here.

  • yes I did sign a CLA.
  • the two patches (attachment:popup.diff and attachment:_HasDropDown.diff) were produced with the unix diff command.
  • indeed, adding a <div> is not necessary (this was the first solution, unfortunately I could not modify the description of this ticket).

I have no tried yet with 1.4, but generally, having various CSS class names for the various popups is *highly* desirable IMO. Makes styling easier.

Note: See TracTickets for help on using tickets.