Skip to content

Commit

Permalink
use a more reliable rtl detection
Browse files Browse the repository at this point in the history
  • Loading branch information
claviska committed May 28, 2024
1 parent c31d4f5 commit 975115a
Showing 13 changed files with 27 additions and 33 deletions.
4 changes: 4 additions & 0 deletions docs/pages/resources/changelog.md
Original file line number Diff line number Diff line change
@@ -12,6 +12,10 @@ Components with the <sl-badge variant="warning" pill>Experimental</sl-badge> bad

New versions of Shoelace are released as-needed and generally occur when a critical mass of changes have accumulated. At any time, you can see what's coming in the next release by visiting [next.shoelace.style](https://next.shoelace.style).

## Next

- Fixed a bug in the submenu controller that prevented submenus from rendering in RTL without explicitly setting `dir` on the parent menu item [#1992]

## 2.15.1

- Fixed a bug in `<sl-radio-group>` where if a click did not contain a `<sl-radio>` it would show a console error. [#2009]
2 changes: 1 addition & 1 deletion src/components/carousel/carousel.component.ts
Original file line number Diff line number Diff line change
@@ -153,7 +153,7 @@ export default class SlCarousel extends ShoelaceElement {
private handleKeyDown(event: KeyboardEvent) {
if (['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown', 'Home', 'End'].includes(event.key)) {
const target = event.target as HTMLElement;
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const isFocusInPagination = target.closest('[part~="pagination-item"]') !== null;
const isNext =
event.key === 'ArrowDown' || (!isRtl && event.key === 'ArrowRight') || (isRtl && event.key === 'ArrowLeft');
2 changes: 1 addition & 1 deletion src/components/details/details.component.ts
Original file line number Diff line number Diff line change
@@ -187,7 +187,7 @@ export default class SlDetails extends ShoelaceElement {
}

render() {
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

return html`
<details
6 changes: 3 additions & 3 deletions src/components/image-comparer/image-comparer.component.ts
Original file line number Diff line number Diff line change
@@ -49,7 +49,7 @@ export default class SlImageComparer extends ShoelaceElement {

private handleDrag(event: PointerEvent) {
const { width } = this.base.getBoundingClientRect();
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

event.preventDefault();

@@ -64,7 +64,7 @@ export default class SlImageComparer extends ShoelaceElement {

private handleKeyDown(event: KeyboardEvent) {
const isLtr = this.localize.dir() === 'ltr';
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

if (['ArrowLeft', 'ArrowRight', 'Home', 'End'].includes(event.key)) {
const incr = event.shiftKey ? 10 : 1;
@@ -96,7 +96,7 @@ export default class SlImageComparer extends ShoelaceElement {
}

render() {
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

return html`
<div
6 changes: 2 additions & 4 deletions src/components/menu-item/menu-item.component.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { classMap } from 'lit/directives/class-map.js';
import { getTextContent, HasSlotController } from '../../internal/slot.js';
import { html } from 'lit';
import { LocalizeController } from '../../utilities/localize.js';
import { property, query } from 'lit/decorators.js';
import { SubmenuController } from './submenu-controller.js';
import { watch } from '../../internal/watch.js';
@@ -67,9 +66,8 @@ export default class SlMenuItem extends ShoelaceElement {
/** Draws the menu item in a disabled state, preventing selection. */
@property({ type: Boolean, reflect: true }) disabled = false;

private readonly localize = new LocalizeController(this);
private readonly hasSlotController = new HasSlotController(this, 'submenu');
private submenuController: SubmenuController = new SubmenuController(this, this.hasSlotController, this.localize);
private submenuController: SubmenuController = new SubmenuController(this, this.hasSlotController);

connectedCallback() {
super.connectedCallback();
@@ -155,7 +153,7 @@ export default class SlMenuItem extends ShoelaceElement {
}

render() {
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const isSubmenuExpanded = this.submenuController.isExpanded();

return html`
16 changes: 4 additions & 12 deletions src/components/menu-item/submenu-controller.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { createRef, ref, type Ref } from 'lit/directives/ref.js';
import { type HasSlotController } from '../../internal/slot.js';
import { html } from 'lit';
import { type LocalizeController } from '../../utilities/localize.js';
import type { ReactiveController, ReactiveControllerHost } from 'lit';
import type SlMenuItem from './menu-item.js';
import type SlPopup from '../popup/popup.js';
@@ -15,17 +14,11 @@ export class SubmenuController implements ReactiveController {
private isPopupConnected = false;
private skidding = 0;
private readonly hasSlotController: HasSlotController;
private readonly localize: LocalizeController;
private readonly submenuOpenDelay = 100;

constructor(
host: ReactiveControllerHost & SlMenuItem,
hasSlotController: HasSlotController,
localize: LocalizeController
) {
constructor(host: ReactiveControllerHost & SlMenuItem, hasSlotController: HasSlotController) {
(this.host = host).addController(this);
this.hasSlotController = hasSlotController;
this.localize = localize;
}

hostConnected() {
@@ -202,8 +195,7 @@ export class SubmenuController implements ReactiveController {
private handlePopupReposition = () => {
const submenuSlot: HTMLSlotElement | null = this.host.renderRoot.querySelector("slot[name='submenu']");
const menu = submenuSlot?.assignedElements({ flatten: true }).filter(el => el.localName === 'sl-menu')[0];
const isRtl = this.localize.dir() === 'rtl';

const isRtl = this.host.matches(':dir(rtl)');
if (!menu) {
return;
}
@@ -267,7 +259,7 @@ export class SubmenuController implements ReactiveController {
}

renderSubmenu() {
const isLtr = this.localize.dir() === 'ltr';
const isRtl = this.host.matches(':dir(rtl)');

// Always render the slot, but conditionally render the outer <sl-popup>
if (!this.isConnected) {
@@ -277,7 +269,7 @@ export class SubmenuController implements ReactiveController {
return html`
<sl-popup
${ref(this.popupRef)}
placement=${isLtr ? 'right-start' : 'left-start'}
placement=${isRtl ? 'left-start' : 'right-start'}
anchor="anchor"
flip
flip-fallback-strategy="best-fit"
2 changes: 1 addition & 1 deletion src/components/popup/popup.component.ts
Original file line number Diff line number Diff line change
@@ -413,7 +413,7 @@ export default class SlPopup extends ShoelaceElement {
//
// Source: https://github.com/floating-ui/floating-ui/blob/cb3b6ab07f95275730d3e6e46c702f8d4908b55c/packages/dom/src/utils/getDocumentRect.ts#L31
//
const isRtl = getComputedStyle(this).direction === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const staticSide = { top: 'bottom', right: 'left', bottom: 'top', left: 'right' }[placement.split('-')[0]]!;

this.setAttribute('data-current-placement', placement);
2 changes: 1 addition & 1 deletion src/components/range/range.component.ts
Original file line number Diff line number Diff line change
@@ -175,7 +175,7 @@ export default class SlRange extends ShoelaceElement implements ShoelaceFormCont
const inputWidth = this.input.offsetWidth;
const tooltipWidth = this.output.offsetWidth;
const thumbSize = getComputedStyle(this.input).getPropertyValue('--thumb-size');
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const percentAsWidth = inputWidth * percent;

// The calculations are used to "guess" where the thumb is located. Since we're using the native range control
6 changes: 3 additions & 3 deletions src/components/rating/rating.component.ts
Original file line number Diff line number Diff line change
@@ -80,7 +80,7 @@ export default class SlRating extends ShoelaceElement {
}

private getValueFromXCoordinate(coordinate: number) {
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const { left, right, width } = this.rating.getBoundingClientRect();
const value = isRtl
? this.roundToPrecision(((right - coordinate) / width) * this.max, this.precision)
@@ -109,7 +109,7 @@ export default class SlRating extends ShoelaceElement {

private handleKeyDown(event: KeyboardEvent) {
const isLtr = this.localize.dir() === 'ltr';
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const oldValue = this.value;

if (this.disabled || this.readonly) {
@@ -214,7 +214,7 @@ export default class SlRating extends ShoelaceElement {
}

render() {
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const counter = Array.from(Array(this.max).keys());
let displayValue = 0;

4 changes: 2 additions & 2 deletions src/components/split-panel/split-panel.component.ts
Original file line number Diff line number Diff line change
@@ -102,7 +102,7 @@ export default class SlSplitPanel extends ShoelaceElement {
}

private handleDrag(event: PointerEvent) {
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

if (this.disabled) {
return;
@@ -223,7 +223,7 @@ export default class SlSplitPanel extends ShoelaceElement {
render() {
const gridTemplate = this.vertical ? 'gridTemplateRows' : 'gridTemplateColumns';
const gridTemplateAlt = this.vertical ? 'gridTemplateColumns' : 'gridTemplateRows';
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const primary = `
clamp(
0%,
6 changes: 3 additions & 3 deletions src/components/tab-group/tab-group.component.ts
Original file line number Diff line number Diff line change
@@ -177,7 +177,7 @@ export default class SlTabGroup extends ShoelaceElement {
// Move focus left or right
if (['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown', 'Home', 'End'].includes(event.key)) {
const activeEl = this.tabs.find(t => t.matches(':focus'));
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

if (activeEl?.tagName.toLowerCase() === 'sl-tab') {
let index = this.tabs.indexOf(activeEl);
@@ -292,7 +292,7 @@ export default class SlTabGroup extends ShoelaceElement {

const width = currentTab.clientWidth;
const height = currentTab.clientHeight;
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

// We can't used offsetLeft/offsetTop here due to a shadow parent issue where neither can getBoundingClientRect
// because it provides invalid values for animating elements: https://bugs.chromium.org/p/chromium/issues/detail?id=920069
@@ -370,7 +370,7 @@ export default class SlTabGroup extends ShoelaceElement {
}

render() {
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

return html`
<div
2 changes: 1 addition & 1 deletion src/components/tree-item/tree-item.component.ts
Original file line number Diff line number Diff line change
@@ -223,7 +223,7 @@ export default class SlTreeItem extends ShoelaceElement {
}

render() {
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');
const showExpandButton = !this.loading && (!this.isLeaf || this.lazy);

return html`
2 changes: 1 addition & 1 deletion src/components/tree/tree.component.ts
Original file line number Diff line number Diff line change
@@ -226,7 +226,7 @@ export default class SlTree extends ShoelaceElement {

const items = this.getFocusableItems();
const isLtr = this.localize.dir() === 'ltr';
const isRtl = this.localize.dir() === 'rtl';
const isRtl = this.matches(':dir(rtl)');

if (items.length > 0) {
event.preventDefault();

2 comments on commit 975115a

@LorenzKahl
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @claviska, I wonder why some other components like Button where left out of this change.
If that was intentional I would like to understand the reason behind this.

Cheers, Lorenz

@claviska
Copy link
Member Author

Choose a reason for hiding this comment

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

Most likely an oversight.

Please sign in to comment.