Skip to content

Commit

Permalink
More resilient directionality checks (#2189)
Browse files Browse the repository at this point in the history
* more resilient directionality checks; fixes #2188

* update check

* prettier

---------

Co-authored-by: Konnor Rogers <[email protected]>
  • Loading branch information
claviska and KonnorRogers authored Oct 10, 2024
1 parent 5a3d46a commit 2c66c85
Show file tree
Hide file tree
Showing 13 changed files with 39 additions and 27 deletions.
1 change: 1 addition & 0 deletions docs/pages/resources/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti

## Next

- Updated all checks for directionality to use `this.localize.dir()` instead of `el.matches(:dir(rtl))` so older browsers don't error out [#2188]
- Added Finnish translations [#2211]
- Added the `.focus` function to `<sl-radio-group>` [#2192]
- Fixed a bug with with `<sl-select>` not respecting its initial value. [#2204]
Expand Down
6 changes: 3 additions & 3 deletions src/components/carousel/carousel.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.matches(':dir(rtl)');
const isRtl = this.localize.dir() === 'rtl';
const isFocusInPagination = target.closest('[part~="pagination-item"]') !== null;
const isNext =
event.key === 'ArrowDown' || (!isRtl && event.key === 'ArrowRight') || (isRtl && event.key === 'ArrowLeft');
Expand Down Expand Up @@ -461,7 +461,7 @@ export default class SlCarousel extends ShoelaceElement {
: clamp(index, 0, slides.length - slidesPerPage);
this.activeSlide = newActiveSlide;

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

// Get the index of the next slide. For looping carousel it adds `slidesPerPage`
// to normalize the starting index in order to ignore the first nth clones.
Expand Down Expand Up @@ -498,7 +498,7 @@ export default class SlCarousel extends ShoelaceElement {
const currentPage = this.getCurrentPage();
const prevEnabled = this.canScrollPrev();
const nextEnabled = this.canScrollNext();
const isLtr = this.matches(':dir(ltr)');
const isLtr = this.localize.dir() === 'rtl';

return html`
<div part="base" class="carousel">
Expand Down
2 changes: 1 addition & 1 deletion src/components/details/details.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export default class SlDetails extends ShoelaceElement {
}

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

return html`
<details
Expand Down
11 changes: 7 additions & 4 deletions src/components/image-comparer/image-comparer.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { clamp } from '../../internal/math.js';
import { classMap } from 'lit/directives/class-map.js';
import { drag } from '../../internal/drag.js';
import { html } from 'lit';
import { LocalizeController } from '../../utilities/localize.js';
import { property, query } from 'lit/decorators.js';
import { styleMap } from 'lit/directives/style-map.js';
import { watch } from '../../internal/watch.js';
Expand Down Expand Up @@ -38,6 +39,8 @@ export default class SlImageComparer extends ShoelaceElement {
static styles: CSSResultGroup = [componentStyles, styles];
static scopedElement = { 'sl-icon': SlIcon };

private readonly localize = new LocalizeController(this);

@query('.image-comparer') base: HTMLElement;
@query('.image-comparer__handle') handle: HTMLElement;

Expand All @@ -46,7 +49,7 @@ export default class SlImageComparer extends ShoelaceElement {

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

event.preventDefault();

Expand All @@ -60,8 +63,8 @@ export default class SlImageComparer extends ShoelaceElement {
}

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

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

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

return html`
<div
Expand Down
4 changes: 3 additions & 1 deletion src/components/menu-item/menu-item.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
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';
Expand Down Expand Up @@ -47,6 +48,7 @@ export default class SlMenuItem extends ShoelaceElement {
};

private cachedTextLabel: string;
private readonly localize = new LocalizeController(this);

@query('slot:not([name])') defaultSlot: HTMLSlotElement;
@query('.menu-item') menuItem: HTMLElement;
Expand Down Expand Up @@ -153,7 +155,7 @@ export default class SlMenuItem extends ShoelaceElement {
}

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

return html`
Expand Down
4 changes: 2 additions & 2 deletions src/components/menu-item/submenu-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +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.host.matches(':dir(rtl)');
const isRtl = getComputedStyle(this.host).direction === 'rtl';
if (!menu) {
return;
}
Expand Down Expand Up @@ -259,7 +259,7 @@ export class SubmenuController implements ReactiveController {
}

renderSubmenu() {
const isRtl = this.host.matches(':dir(rtl)');
const isRtl = getComputedStyle(this.host).direction === 'rtl';

// Always render the slot, but conditionally render the outer <sl-popup>
if (!this.isConnected) {
Expand Down
4 changes: 3 additions & 1 deletion src/components/popup/popup.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { arrow, autoUpdate, computePosition, flip, offset, platform, shift, size } from '@floating-ui/dom';
import { classMap } from 'lit/directives/class-map.js';
import { html } from 'lit';
import { LocalizeController } from '../../utilities/localize.js';
import { offsetParent } from 'composed-offset-position';
import { property, query } from 'lit/decorators.js';
import componentStyles from '../../styles/component.styles.js';
Expand Down Expand Up @@ -56,6 +57,7 @@ export default class SlPopup extends ShoelaceElement {

private anchorEl: Element | VirtualElement | null;
private cleanup: ReturnType<typeof autoUpdate> | undefined;
private readonly localize = new LocalizeController(this);

/** A reference to the internal popup container. Useful for animating and styling the popup with JavaScript. */
@query('.popup') popup: HTMLElement;
Expand Down Expand Up @@ -413,7 +415,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 = this.matches(':dir(rtl)');
const isRtl = this.localize.dir() === 'rtl';
const staticSide = { top: 'bottom', right: 'left', bottom: 'top', left: 'right' }[placement.split('-')[0]]!;

this.setAttribute('data-current-placement', placement);
Expand Down
2 changes: 1 addition & 1 deletion src/components/range/range.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.matches(':dir(rtl)');
const isRtl = this.localize.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
Expand Down
11 changes: 7 additions & 4 deletions src/components/rating/rating.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { clamp } from '../../internal/math.js';
import { classMap } from 'lit/directives/class-map.js';
import { eventOptions, property, query, state } from 'lit/decorators.js';
import { html } from 'lit';
import { LocalizeController } from '../../utilities/localize.js';
import { styleMap } from 'lit/directives/style-map.js';
import { unsafeHTML } from 'lit/directives/unsafe-html.js';
import { watch } from '../../internal/watch.js';
Expand Down Expand Up @@ -35,6 +36,8 @@ export default class SlRating extends ShoelaceElement {
static styles: CSSResultGroup = [componentStyles, styles];
static dependencies = { 'sl-icon': SlIcon };

private readonly localize = new LocalizeController(this);

@query('.rating') rating: HTMLElement;

@state() private hoverValue = 0;
Expand Down Expand Up @@ -77,7 +80,7 @@ export default class SlRating extends ShoelaceElement {
}

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

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

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

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

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

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

if (this.disabled) {
return;
Expand Down Expand Up @@ -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.matches(':dir(rtl)');
const isRtl = this.localize.dir() === 'rtl';
const primary = `
clamp(
0%,
Expand Down
9 changes: 4 additions & 5 deletions src/components/tab-group/tab-group.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,13 @@ export default class SlTabGroup extends ShoelaceElement {
static styles: CSSResultGroup = [componentStyles, styles];
static dependencies = { 'sl-icon-button': SlIconButton, 'sl-resize-observer': SlResizeObserver };

private readonly localize = new LocalizeController(this);

private activeTab?: SlTab;
private mutationObserver: MutationObserver;
private resizeObserver: ResizeObserver;
private tabs: SlTab[] = [];
private focusableTabs: SlTab[] = [];
private panels: SlTabPanel[] = [];
private readonly localize = new LocalizeController(this);

@query('.tab-group') tabGroup: HTMLElement;
@query('.tab-group__body') body: HTMLSlotElement;
Expand Down Expand Up @@ -182,7 +181,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.matches(':dir(rtl)');
const isRtl = this.localize.dir() === 'rtl';
let nextTab: null | SlTab = null;

if (activeEl?.tagName.toLowerCase() === 'sl-tab') {
Expand Down Expand Up @@ -302,7 +301,7 @@ export default class SlTabGroup extends ShoelaceElement {

const width = currentTab.clientWidth;
const height = currentTab.clientHeight;
const isRtl = this.matches(':dir(rtl)');
const isRtl = this.localize.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
Expand Down Expand Up @@ -434,7 +433,7 @@ export default class SlTabGroup extends ShoelaceElement {
}

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

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

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

return html`
Expand Down
6 changes: 4 additions & 2 deletions src/components/tree/tree.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { clamp } from '../../internal/math.js';
import { html } from 'lit';
import { LocalizeController } from '../../utilities/localize.js';
import { property, query } from 'lit/decorators.js';
import { watch } from '../../internal/watch.js';
import componentStyles from '../../styles/component.styles.js';
Expand Down Expand Up @@ -89,6 +90,7 @@ export default class SlTree extends ShoelaceElement {
private lastFocusedItem: SlTreeItem | null;
private mutationObserver: MutationObserver;
private clickTarget: SlTreeItem | null = null;
private readonly localize = new LocalizeController(this);

constructor() {
super();
Expand Down Expand Up @@ -222,8 +224,8 @@ export default class SlTree extends ShoelaceElement {
}

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

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

0 comments on commit 2c66c85

Please sign in to comment.