#15531 closed enhancement (fixed)
Button: control of Base text direction
Reported by: | wajnberg | Owned by: | Douglas Hays |
---|---|---|---|
Priority: | undecided | Milestone: | 1.9 |
Component: | Dijit - Form | Version: | 1.7.2 |
Keywords: | Cc: | sashash | |
Blocked By: | Blocking: | #15734 |
Description (last modified by )
Support for textDir property in Button, ToggleButton?, DropDownButton? and ComboButton?, same as for form widget. The core support for base text direction (via textDir) was introduced via #12367.
Attachments (2)
Change History (12)
comment:1 Changed 9 years ago by
Description: | modified (diff) |
---|---|
Milestone: | tbd → 2.0 |
comment:2 Changed 8 years ago by
Milestone: | 2.0 → 1.9 |
---|---|
Summary: | Button: control of Base text direction in → Button: control of Base text direction |
comment:3 Changed 8 years ago by
The patch can be simplified by removing the second argument to applyTextDir() (see [29559]).
Also, same as #15654, likely the _setTextDirAttr() functionality in Button.js should instead be moved into _BidiSupport::_setTextDirAttr(), by either generalizing that existing method to apply to this.containerNode||this.focusNode
(if this.displayNode is not set), or by perhaps setting up a new data-dojo-attach-point called textDirNode (or something like that).
Changed 8 years ago by
Attachment: | 15531_15654.patch added |
---|
comment:4 Changed 8 years ago by
Cc: | sashash added |
---|
I attached a patch that needs review/testing. I changed the MenuItem/textDir references from focusNode to containerNode, and created a generalized _setTextDirAttr in _BidiSupport.js
comment:5 Changed 8 years ago by
I forget why/when that code to set align is necessary, and why it would be needed exclusively for this.displayNode (but not this.containerNode etc.).
Also, one minor thing is that (at least in theory) you shouldn't need the if(this.textDir === "auto")
switch because I updated _BidiSupport:: applyTextDir() to not do anything when textDir=="", and hopefully not do anything unwanted when textDir=="ltr" or "rtl".
But other than that the patch looks good to me.
comment:6 Changed 8 years ago by
Blocking: | 15734 added |
---|
(In #15734) Needs #15531 for StackContainer? buttons.
comment:8 Changed 8 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
I'm getting failures in the menu test after this. One problem is that you didn't update MenuBarItem.html to have a textDirNode attach point, but even after fixing that I get failures.
I am noticing a lot of duplication across dijit, with lots of _setTextDirAttr methods that are basically the same. It seems like it would be better to have a single method in _BidiSupport.js that handled simple cases like Button and MenuItem.