Skip to content

Widgets: Add RTL support #1682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions demos/accordion/default-rtl.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>jQuery UI Accordion - Default functionality</title>
<link rel="stylesheet" href="../../themes/base/all.css">
<link rel="stylesheet" href="../demos.css">
<script src="../../external/requirejs/require.js"></script>
<script src="../bootstrap.js">
var icons = {
header: "ui-icon-triangle-1-w"
};
$( "#accordion" ).accordion({
icons: icons
});
</script>
</head>
<body dir="rtl">

<div id="accordion">
<h3>Section 1</h3>
<div>
<p>
Mauris mauris ante, blandit et, ultrices a, suscipit eget, quam. Integer
ut neque. Vivamus nisi metus, molestie vel, gravida in, condimentum sit
amet, nunc. Nam a nibh. Donec suscipit eros. Nam mi. Proin viverra leo ut
odio. Curabitur malesuada. Vestibulum a velit eu ante scelerisque vulputate.
</p>
</div>
<h3>Section 2</h3>
<div>
<p>
Sed non urna. Donec et ante. Phasellus eu ligula. Vestibulum sit amet
purus. Vivamus hendrerit, dolor at aliquet laoreet, mauris turpis porttitor
velit, faucibus interdum tellus libero ac justo. Vivamus non quam. In
suscipit faucibus urna.
</p>
</div>
<h3>Section 3</h3>
<div>
<p>
Nam enim risus, molestie et, porta ac, aliquam ac, risus. Quisque lobortis.
Phasellus pellentesque purus in massa. Aenean in pede. Phasellus ac libero
ac tellus pellentesque semper. Sed ac felis. Sed commodo, magna quis
lacinia ornare, quam ante aliquam nisi, eu iaculis leo purus venenatis dui.
</p>
<ul>
<li>List item one</li>
<li>List item two</li>
<li>List item three</li>
</ul>
</div>
<h3>Section 4</h3>
<div>
<p>
Cras dictum. Pellentesque habitant morbi tristique senectus et netus
et malesuada fames ac turpis egestas. Vestibulum ante ipsum primis in
faucibus orci luctus et ultrices posuere cubilia Curae; Aenean lacinia
mauris vel est.
</p>
<p>
Suspendisse eu nisl. Nullam ut libero. Integer dignissim consequat lectus.
Class aptent taciti sociosqu ad litora torquent per conubia nostra, per
inceptos himenaeos.
</p>
</div>
</div>

<div class="demo-description">
<p>
Click headers to expand/collapse content that is broken into logical sections, much like tabs.
Optionally, toggle sections open/closed on mouseover.
</p>
<p>
The underlying HTML markup is a series of headers (H3 tags) and content divs so the content is
usable without JavaScript.
</p>
</div>
</body>
</html>
1 change: 1 addition & 0 deletions demos/accordion/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

<ul>
<li><a href="default.html">Default functionality</a></li>
<li><a href="default-rtl.html">Default functionality(RTL)</a></li>
<li><a href="fillspace.html">Fill space</a></li>
<li><a href="no-auto-height.html">No auto height</a></li>
<li><a href="collapsible.html">Collapse content</a></li>
Expand Down
52 changes: 52 additions & 0 deletions demos/autocomplete/default-rtl.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>jQuery UI Autocomplete - Default functionality</title>
<link rel="stylesheet" href="../../themes/base/all.css">
<link rel="stylesheet" href="../demos.css">
<script src="../../external/requirejs/require.js"></script>
<script src="../bootstrap.js">
var availableTags = [
"ActionScript",
"AppleScript",
"Asp",
"BASIC",
"C",
"C++",
"Clojure",
"COBOL",
"ColdFusion",
"Erlang",
"Fortran",
"Groovy",
"Haskell",
"Java",
"JavaScript",
"Lisp",
"Perl",
"PHP",
"Python",
"Ruby",
"Scala",
"Scheme"
];
$( "#tags" ).autocomplete({
source: availableTags
});
</script>
</head>
<body>

<div class="ui-widget">
<label for="tags">Tags: </label>
<input id="tags" dir="rtl">
</div>

<div class="demo-description">
<p>The Autocomplete widgets provides suggestions while you type into the field. Here the suggestions are tags for programming languages, give "ja" (for Java or JavaScript) a try.</p>
<p>The datasource is a simple JavaScript array, provided to the widget using the source-option.</p>
</div>
</body>
</html>
1 change: 1 addition & 0 deletions demos/autocomplete/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

<ul>
<li><a href="default.html">Default functionality</a></li>
<li><a href="default-rtl.html">Default functionality(rtl)</a></li>
<li><a href="remote.html">Remote datasource</a></li>
<li><a href="remote-with-cache.html">Remote with caching</a></li>
<li><a href="remote-jsonp.html">Remote JSONP datasource</a></li>
Expand Down
2 changes: 1 addition & 1 deletion themes/base/accordion.css
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
cursor: pointer;
position: relative;
margin: 2px 0 0 0;
padding: .5em .5em .5em .7em;
padding: .5em;
font-size: 100%;
}
.ui-accordion .ui-accordion-content {
Expand Down
3 changes: 3 additions & 0 deletions themes/base/autocomplete.css
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,6 @@
left: 0;
cursor: default;
}
.ui-autocomplete-menu-rtl {
direction: rtl;
}
4 changes: 4 additions & 0 deletions ui/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,10 @@ $.Widget.prototype = {
return {};
},

isRtl: function() {
return this.element.css( "direction" ) === "rtl";
},

_getCreateEventData: $.noop,

_create: $.noop,
Expand Down
6 changes: 5 additions & 1 deletion ui/widgets/autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,9 @@ $.widget( "ui.autocomplete", {
.menu( "instance" );

this._addClass( this.menu.element, "ui-autocomplete", "ui-front" );
if ( this.isRtl() ) {
this._addClass( this.menu.element, "ui-autocomplete-menu-rtl" );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it should just be a generic .ui-rtl class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed on generic but i wonder should we even use a class or should we set the attribute?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I guess it depends on whether this will be used exclusively on generated content, which it might. If it is, then just setting the attribute is fine since we don't need to clean it up. Cleaning up the class would be easier.

}
this._on( this.menu.element, {
mousedown: function( event ) {

Expand Down Expand Up @@ -539,9 +542,10 @@ $.widget( "ui.autocomplete", {
// Size and position menu
ul.show();
this._resizeMenu();
var pos = this.isRtl() ? { my: "right top", at: "right bottom", collision: "none" } : this.options.position;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be hard-coded like this. It makes the option impossible to use.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I create _getPositionRtl: function( position ) which replace every left with right & vice versa when this.isRtl() == true ?
If so, should I put it in widget.js as it might be used within others widgets ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you should assume the user has the option set to what they actually want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do anything with position, we would change the default to null, and on create, we would set a value based on the direction if the value is still null.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be consistent with our null value options approach in other places

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The browser will not change that, but someone have to change it to be acceptable for bidi-users.
You are suggesting that the user should make the changes himself. But I prefer the changes to be made by the library.
So instead of letting every user writing another version of his app for rtl-layout, the library will do that for him by just setting dir=rtl. And he still have the ability to disable or override that behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you, we have to respect the user's choice & that is what we actually do in ltr-layout.
The user will always get what he expects in ltr-layout. But when he choose to enable mirroring(by setting dir=rtl), the library should do the magic for him with no or with minimal modification from his side.
The pros of this approach is:
1- The user don't have to write another app or make any major changes to enable ui mirroring.
2- Users don't have to have a deep knowledge about mirroring to make their apps mirrored correctly.
3- The user still have the ability to disable or override that behavoir.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The browser will not change that, but someone have to change it to be acceptable for bidi-users.

That not always true you can choose to have something in the same position regardless of your layout this is application specific and gives developers a way to force position regardless of direction if thats what their app requires.

The user still have the ability to disable or override that behavoir.

Please explain to me with your approach how you would set the position to be the same REGARDLESS of direction. As far as i can tell this is not possible with your approach. Also if someone was making a RTL first app do you really expect them to set position left when they really want right? this seems completely wrong an counter intuitive and will only lead to confusion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also very easy to write a plugin that does the mirroring for you. Widget functionality is never a black and white choice of the built-in or implemented as a one-off for every single user. Creating a position mirroring extension would allow users to have the mirroring logic without writing it themselves, but I still don't agree that automatic mirroring for position is correct.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I should revert it back.

ul.position( $.extend( {
of: this.element
}, this.options.position ) );
}, pos ) );

if ( this.options.autoFocus ) {
this.menu.next();
Expand Down