Skip to content
This repository was archived by the owner on Jan 3, 2024. It is now read-only.

Adds new tab component tabbed including child components, for #535#539

Merged
PVince81 merged 1 commit intomasterfrom
feature/a11y-documentation/tab
Nov 26, 2019
Merged

Adds new tab component tabbed including child components, for #535#539
PVince81 merged 1 commit intomasterfrom
feature/a11y-documentation/tab

Conversation

@marcus-herrmann
Copy link
Copy Markdown
Contributor

Establishes:

  • New tab component oc-tabbed
  • New tab-item component oc-tabbed-tab
  • New panel-item component oc-tabbed-panel

Structure according to the briefing by @PVince81

Keyboard behaviour, ARIA roles, properties and states according to the Authoring Practice

Intention for establishing new components: Trying to avoid to break phoenix by using the oc-tab-item namespace since the new component is much more complex than the existing one.

@PVince81
Copy link
Copy Markdown
Contributor

@PVince81
Copy link
Copy Markdown
Contributor

code looks fine, as discussed.

please submit a PR in Phoenix that works with this PR here, this will confirm that the new tabbed component can work "on the field"

@marcus-herrmann marcus-herrmann force-pushed the feature/a11y-documentation/tab branch 5 times, most recently from b75d520 to 1e84896 Compare November 21, 2019 07:00
this.changeTab(e.target.id)
},
keydownHandler: function(e) {
const activeElem = this.$refs.tablist.querySelector(`#${this.activeTab}`)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

using query selector feels do un-vuejs-ish ... this is what vuejs has refs for ..... or am I missing anything?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can't really use a ref here since the name is dynamic, I think

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

indeed, seems not possible. refs are limited to the current component: https://stackoverflow.com/a/42814625

Copy link
Copy Markdown
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81 PVince81 merged commit 1b54d1f into master Nov 26, 2019
@delete-merged-branch delete-merged-branch bot deleted the feature/a11y-documentation/tab branch November 26, 2019 13:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants