jquery-ui
jquery-ui copied to clipboard
jquery
jquery-ui:Base text direction support
As per discussion in https://bugs.jqueryui.com/ticket/14477, current code submit contains Base text direction support for UI widgets. New option is introduced for relevant widgets 'textDir' with values: 'ltr' - Left-to-right text direction 'rtl' - Right-to-left text direction 'auto' - Contextual text direction that gets resolved to either 'ltr' or 'rtl' according to first strong character ('rtl' if it is Arabic/Hebrew character, 'ltr' otherwise). Text direction option shouldn't affect the mirroring aspects of widgets having assimmetric GUI, it only impacts the text direction of contained text. The corresponding demo textDir.html illustrates the issue for all relevant widgets. When current approach will be approved (or modified according reviewer remaarks) documentation and unitests will be provided.
By analyzing the blame information on this pull request, we identified @arschmitz, @scottgonzalez and @jzaefferer to be potential reviewers
Thanks. See also #1682.
Just FYI: We won't have any time to discuss the approach until after our releases are done, which is why #1682 has been sitting for a long time.
No problem. Will be waiting for your feedback / comments.
One clarification with your kind permission:
#1682 is about fixing issues with GUI mirroring (relevant for several widgets). GUI mirroring is strongly associated with translation of UI to Arabic / Hebrew. At the same time, GUI mirroring is mostly concerned with graphical layout of screen and widget.
As opposed to that, this specific PR is concerned with text only (which can appear as part of widget). Thus this PR is completely orthogonal to GUI direction and #1682 .
Just FYI. My team and our colleagues from Egypt Bidi center (who authored #1682) work together to address variety of Bidi requirements in open source tools / libraries.
I have not reviewed this at all yet simply read the discription. Just one minor thing i noticed from that is any time we have an "auto" option like this that determines the value used based on DOM etc, we always use the special value null to denote it. so in this case the 3 values should be "ltr", "rtl", and null. null should be the default value. When the value null is used and rtl or ltr chosen based on markup during init, the option value should be set as well. In this way when a developer checks the value of the option it should always return rtl or ltr and never null
@scottgonzalez may I kindly inquire which release are we waiting for to start the review ? I believe 3.2 was out in March 2017.
Let's focus on a single widget to have our discussions. I'll pick button since it's the first widget in the textDir.html demo and is fairly simple. Please explain exactly what the button widget is accounting for with text direction that isn't already handled natively by dir="rtl".
@scottgonzalez button is not the best example to illustrate the benefit of textDir parameter. This is mostly since button has horizontally symmetrical graphics (just a rectangular area without any shade) and thus it looks (from graphical perspective) exactly the same with both LTR and RTL directions. However even for the button there is a minor benefit. Please observe that textDir is meant to affect text only. Currently if you specify dir="ltr" or dir="rtl" for button you don't cover auto direction (aka contextual or first stong) for text label displayed as part of the button. You can specify dir="auto" for button but this won't work at least not on IE. Thus having textDir even in the case of button has some benefit.
In general textDir has striking benefit for widgets which are horizontally asymmetrical. For example, combo box, list box (with scroll bar), check box, tree, grid etc. When you want to flip horizontally such widgets you usually specify dir="ltr" or dir="rtl". However, this by itself does not allow you to enforce text direction for text displayed in such widgets independently from GUI direction. For example if you specify dir="rtl" for combo box, all text inside such combo box will be displayed with RTL direction. However if you wish this text to be displayed with LTR direction you have no such option. Having textDir parameter allows you to control text direction of text inside a widget independently from direction of widget graphics.
For the sake of example, please observe the behavior of native controls on iOS / Android platforms. Text in such controls is displayed by default with textDir = "auto" regardless of direction of control's graphics (dir ="ltr" or dir ="rtl"). By the way majority of desktop technologies (including native mobile libraries) allow to control direction of text independently from widget (or GUI) direction. For example:
- Direction of GUI (or widget) is called by Android layoutDirection (https://developer.android.com/reference/android/view/View.html#setLayoutDirection%28int%29) . It is equivalent to HTML dir specified for a widget.
- Text direction (for text displayed inside a widget) is called by Android textDirection (https://developer.android.com/reference/android/view/View.html#setTextDirection%28int%29). It is equivalent to HTML dir specified on the text inline element itself.
Thanks for the explanation on the two different direction settings. Now that I have a better understanding of the specific goal trying to be accomplished, I will take another look at the changes.
@ashensis You can just push updates to this branch and keep the PR open.
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
@scottgonzalez All comments in initial round of review had been addressed, please have a look
@scottgonzalez Are you ok with @ashensis changes ? Did he address all your comments ? Is anything else needed to allow his code to be merged ?
@arschmitz and @jzaefferer who is in charge to review this PR . @scottgonzales is not answering a long time