Skip to content

Commit

Permalink
fix: remove firstRender attribute and revert to initial autofocus app… (
Browse files Browse the repository at this point in the history
#1631)

* fix: remove firstRender attribute and revert to initial autofocus approach
  • Loading branch information
felix-ico authored Mar 15, 2023
1 parent 38ed163 commit 643245b
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 35 deletions.
15 changes: 8 additions & 7 deletions packages/components/src/components/tab-header/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@

## Properties

| Property | Attribute | Description | Type | Default |
| ---------- | ---------- | ------------------------------------------------------------------------------------- | -------------------- | ----------- |
| `disabled` | `disabled` | True for a disabled Tabnavigation | `boolean` | `false` |
| `selected` | `selected` | | `boolean` | `undefined` |
| `size` | `size` | (optional) size | `"large" \| "small"` | `'small'` |
| `small` | `small` | <span style="color:red">**[DEPRECATED]**</span> - size should replace small<br/><br/> | `boolean` | `false` |
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |
| Property | Attribute | Description | Type | Default |
| ----------- | ------------ | ------------------------------------------------------------------------------------- | -------------------- | ----------- |
| `autoFocus` | `auto-focus` | (optional) autoFocus | `boolean` | `undefined` |
| `disabled` | `disabled` | True for a disabled Tabnavigation | `boolean` | `false` |
| `selected` | `selected` | | `boolean` | `undefined` |
| `size` | `size` | (optional) size | `"large" \| "small"` | `'small'` |
| `small` | `small` | <span style="color:red">**[DEPRECATED]**</span> - size should replace small<br/><br/> | `boolean` | `false` |
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |


----------------------------------------------
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/components/tab-header/tab-header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export class TabHeader {
@Prop() small?: boolean = false;
/** (optional) size */
@Prop() size: 'small' | 'large' = 'small';
/** (optional) autoFocus */
@Prop() autoFocus?: boolean;
/** (optional) Injected CSS styles */
@Prop() styles?: string;
@Prop() selected: boolean;
Expand All @@ -50,8 +52,7 @@ export class TabHeader {
if (newValue === true) {
// Having focus on the host element, and not on inner elements,
// is required because screen readers.
const firstRender = this.hostElement.getAttribute('first-render');
if (!firstRender) {
if (this.autoFocus) {
this.hostElement.focus();
}
}
Expand Down
11 changes: 6 additions & 5 deletions packages/components/src/components/tab-nav/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@

## Properties

| Property | Attribute | Description | Type | Default |
| -------- | --------- | ------------------------------------------------------------------------------------- | -------------------- | ----------- |
| `size` | `size` | (optional) size | `"large" \| "small"` | `'small'` |
| `small` | `small` | <span style="color:red">**[DEPRECATED]**</span> - size should replace small<br/><br/> | `boolean` | `false` |
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |
| Property | Attribute | Description | Type | Default |
| ----------- | ------------ | ------------------------------------------------------------------------------------- | -------------------- | ----------- |
| `autoFocus` | `auto-focus` | (optional) autoFocus | `boolean` | `false` |
| `size` | `size` | (optional) size | `"large" \| "small"` | `'small'` |
| `small` | `small` | <span style="color:red">**[DEPRECATED]**</span> - size should replace small<br/><br/> | `boolean` | `false` |
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |


----------------------------------------------
Expand Down
26 changes: 5 additions & 21 deletions packages/components/src/components/tab-nav/tab-nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ export class TabNav {
@Prop() small?: boolean = false;
/** (optional) size */
@Prop() size: 'small' | 'large' = 'small';
/** (optional) autoFocus */
@Prop() autoFocus: boolean = false;
/** (optional) Injected CSS styles */
@Prop() styles?: string;

@Listen('click')
handleClick(event: MouseEvent) {
this.removeFirstRenderAttr();

// workaround for slotted icons
const targetHTMLElement = event.target as HTMLElement;
const targetTag = targetHTMLElement.tagName.toLowerCase();
Expand Down Expand Up @@ -69,8 +69,6 @@ export class TabNav {

@Listen('keydown')
handleKeydown(event: KeyboardEvent) {
this.removeFirstRenderAttr();

const target = event.target as HTMLScaleTabHeaderElement;
let nextTab;

Expand Down Expand Up @@ -103,13 +101,6 @@ export class TabNav {
this.selectTab(nextTab);
}

removeFirstRenderAttr() {
const tabs = this.getAllEnabledTabs();
tabs.forEach((tab) => {
tab.removeAttribute('first-render');
});
}

connectedCallback() {
if (!this.el.hasAttribute('role')) {
this.el.setAttribute('role', 'tablist');
Expand Down Expand Up @@ -173,22 +164,15 @@ export class TabNav {

linkPanels() {
const tabs = this.getAllEnabledTabs();
const selectedTab = tabs.find((x) => x.selected);
const selectedTab = tabs.find((x) => x.selected) || tabs[0];

tabs.forEach((tab) => {
const panel = tab.nextElementSibling;
tab.setAttribute('aria-controls', panel.id);
tab.setAttribute('auto-focus', this.autoFocus.toString());
panel.setAttribute('aria-labelledby', tab.id);
if (!selectedTab) {
// we pass this down to tab-header to prevent the first element to be focused on first render (a11y)
tab.setAttribute('first-render', 'true');
}
});
if (selectedTab) {
this.selectTab(selectedTab);
} else {
this.selectTab(tabs[0]);
}
this.selectTab(selectedTab);
}

reset() {
Expand Down

0 comments on commit 643245b

Please sign in to comment.