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.

Sign up for GitHub

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

Widgets: Add RTL support #1682

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2c9539a
Accordion: Adding RTL support
AhmedMustafa Mar 4, 2016
34004ae
Accordion: updating rtl support in ui.accordion
AhmedMustafa Mar 6, 2016
8f86527
Autocomplete: Adding ui-mirroring support
AhmedMustafa Mar 7, 2016
8ba1210
Autocomplete: ui mirroring support
AhmedMustafa Mar 15, 2016
1946111
Button: Adding ui mirroring support
AhmedMustafa Mar 15, 2016
1a47821
Controlgroup: Adding ui mirroring support
AhmedMustafa Mar 16, 2016
6a00fb0
Checkboxradio: Adding rtl demo
AhmedMustafa Mar 16, 2016
23cc693
Dialog: Adding ui mirroring support
AhmedMustafa Mar 17, 2016
543aaea
Draggable: Adding ui mirroring demo
AhmedMustafa Mar 17, 2016
e3581d0
Droppable: Adding ui mirroring demo
AhmedMustafa Mar 17, 2016
fc01eb6
Menu: Adding ui mirroring support
AhmedMustafa Mar 19, 2016
43b3c2f
Progressbar: Adding ui mirroring demo
AhmedMustafa Mar 19, 2016
d1f0301
Selectable: Adding ui mirroring demo
AhmedMustafa Mar 19, 2016
10deed0
Selectmenu: Adding ui mirroring support
AhmedMustafa Mar 19, 2016
6999e35
Spinner: Adding ui mirroring support
AhmedMustafa Mar 19, 2016
0a71279
Tabs: Adding ui mirroring support
AhmedMustafa Mar 19, 2016
c92c299
Slider: Adding ui mirroring support
AhmedMustafa Mar 20, 2016
1a1f8b7
Resizable: Adding ui mirroring support
AhmedMustafa Mar 22, 2016
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
76 changes: 76 additions & 0 deletions demos/accordion/default-rtl.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<!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">
$( "#accordion" ).accordion();
</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
View file
Open in desktop
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
11 changes: 11 additions & 0 deletions themes/base/accordion.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,14 @@
border-top: 0;
overflow: auto;
}

/* RTL support */
.ui-accordion-rtl {
direction: rtl;
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to exist? The direction is already set or we wouldn't be adding this class.

Copy link
Author

Choose a reason for hiding this comment

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

That is correct for the current case. But it would be required if there is a parent widget with dir ='rtl' & its child widget exists somewhere in the DOM with no dirattribute (e.g. autocomplete & menu).
I know it doesn't make any sense her, but I added it in case we need it at some point & for the mirroring support to be consistent across all the widgets.

Copy link
Member

Choose a reason for hiding this comment

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

We would never need this for accordion, so it doesn't make sense to add it.

}
.ui-accordion-rtl .ui-accordion .ui-accordion-header {
Copy link
Member

Choose a reason for hiding this comment

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

This is overly specific (and actually wrong). .ui-accordion-rtl .ui-accordion-header is what you actually want.

I'd personally prefer just changing the rule to padding: .5em and not having anything for RTL here.

Copy link
Member

Choose a reason for hiding this comment

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

I'd personally prefer just changing the rule to padding: .5em and not having anything for RTL here.

+1

Copy link
Author

Choose a reason for hiding this comment

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

+1

padding: .5em .7em .5em .5em;
}
.ui-accordion-header-iconRtl.ui-icon {
transform: scaleX(-1);
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 wrong. The user has full control over what icons are used via the icons option.

Copy link
Author

Choose a reason for hiding this comment

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

The support aims to make the changes required to mirror the app as minimal as possible (just set dir='rtl').
So I think it is better for the user to write his app once & just change one attribute (dir) to get his app mirrored.

Copy link
Member

Choose a reason for hiding this comment

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

I very much disagree with this. This penalizes every user who uses RTL and a proper non-symmetrical icon. The documentation would have to tell users to intentionally build their icons backward or override our CSS.

Copy link
Member

Choose a reason for hiding this comment

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

Also many icons should not be mirrored for example play icons fastforward rewind etc all should never be mirrored mirroring is specific to the icon chosen.

Copy link
Author

Choose a reason for hiding this comment

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

that makes sense

}
Copy link
Member

Choose a reason for hiding this comment

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

missing new line at end of file

4 changes: 4 additions & 0 deletions ui/widget.js
View file
Open in desktop
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: 6 additions & 0 deletions ui/widgets/accordion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ return $.widget( "ui.accordion", {
var options = this.options;

this.prevShow = this.prevHide = $();
if ( this.isRtl() ) {
this._addClass( ".ui-accordion-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 shouldn't have a leading dot. I'm also not sure it should be a keyed class.

}
this._addClass( "ui-accordion", "ui-widget ui-helper-reset" );
this.element.attr( "role", "tablist" );

Expand Down Expand Up @@ -108,6 +111,9 @@ return $.widget( "ui.accordion", {

if ( icons ) {
icon = $( "<span>" );
if ( this.isRtl() ) {
this._addClass( icon, "ui-accordion-header-iconRtl" );
}
this._addClass( icon, "ui-accordion-header-icon", "ui-icon " + icons.header );
icon.prependTo( this.headers );
children = this.active.children( ".ui-accordion-header-icon" );
Expand Down