Add focus option to selectTab method#119
Conversation
Allow an optional, second `options` argument to `selectTab()`. Add support for a `focus` option which controls whether the newly selected tab is given focus. The default is true, matching previous behavior without the option. Document the `selectTab()` method in the readme.
| set selectedTabIndex(i: number) { | ||
| this.selectTab(i) | ||
| } |
There was a problem hiding this comment.
As requested I have not changed any existing functionality. However, I might suggest changing this (undocumented, untested) selectedTabIndex setter to not change focus. As a custom component user, I would find it surprising that setting someTabContainer.selectedTabIndex = 3 would have a side effect of moving focus. (Just my opinion.)
set selectedTabIndex(i: number) {
- this.selectTab(i)
+ this.selectTab(i, {focus: false})
}
francinelucca
left a comment
francinelucca
left a comment
There was a problem hiding this comment.
looks good! let's keep the selectedTabIndex setter behavior as-is. I fear changing it would constitute major-breaking behavior.
|
Incidentally, the generated The CEM isn't included in the npm package, so I just ignored it for this PR. (Let me know if I should open an issue or just continue ignoring it.) To see what I mean, in a fresh clone of the repo: # Install *exactly* package-lock.json:
npm ci
npm build
git diffResults: diff --git a/custom-elements.json b/custom-elements.json
index 5772e8e..8e6e938 100644
--- a/custom-elements.json
+++ b/custom-elements.json
@@ -9,16 +9,16 @@
{
"kind": "variable",
"name": "TabContainerChangeEvent",
- "default": "_TabContainerChangeEvent"
+ "default": "class extends Event {\n static"
},
{
"kind": "variable",
"name": "TabContainerElement",
- "default": "_TabContainerElement"
+ "default": "class extends HTMLElement {\n static"
},
{
"kind": "variable",
- "name": "dist_default",
+ "name": "index_default",
"default": "TabContainerElement"
}
],
@@ -43,7 +43,7 @@
"kind": "js",
"name": "default",
"declaration": {
- "name": "dist_default",
+ "name": "index_default",
"module": "dist/bundle.js"
}
} |
2 participants
Add
focusoption toselectTab()method. By default, sets focus to newly selected tab as in existing behavior. With{focus: false}does not change focus.Fixes #117.