-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Widgets: Add RTL support #1682
Changes from 3 commits
2c9539a
34004ae
8f86527
8ba1210
1946111
1a47821
6a00fb0
23cc693
543aaea
e3581d0
fc01eb6
43b3c2f
d1f0301
10deed0
6999e35
0a71279
c92c299
1a1f8b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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> |
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> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,3 +14,6 @@ | |
left: 0; | ||
cursor: default; | ||
} | ||
.ui-autocomplete-menu-rtl { | ||
direction: rtl; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" ); | ||
} | ||
this._on( this.menu.element, { | ||
mousedown: function( event ) { | ||
|
||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I create There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we do anything with position, we would change the default to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be consistent with our There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.