Enhancement: Tab to be set from outside, Keyboard navigation #741
Conversation
|
|
--STORYBOOK-PREVIEW--
|
|
--EXAMPLE-APPS-PREVIEW--
|
|
Checked out the canary-build and as far as I can tell, the dynamic header-values and setActiveTab improvements work as expected. Looking forward to its merge 😃 |
verena-ifx
changed the title
|
Is this PR done? |
|
General note:
|
| @@ -23,18 +23,20 @@ export class Checkbox { | |||
| if (!this.disabled) { | |||
| if (this.inputElement.indeterminate) { | |||
| this.internalValue = true; | |||
| this.value = this.internalValue; | |||
There was a problem hiding this comment.
This should be reversed. You want to assign the value to the internalValue.
| this.indeterminate = false; | ||
| } else { | ||
| this.internalValue = !this.internalValue; | ||
| this.value = this.internalValue; |
There was a problem hiding this comment.
Same as above. Assign the value to the internalValue.
There was a problem hiding this comment.
both need to be in sync, this is why.
There was a problem hiding this comment.
But if you need to reassign the value to the internalValue, then the value Prop has been changed externally, and therefore both will be synced by assigning the value to the internalValue.
| @Watch('value') | ||
| valueChanged(newValue: boolean, oldValue: boolean) { | ||
| console.log("watch"); |
There was a problem hiding this comment.
Did somebody forgot to remove the console.log? :)
| // Reset to previously active tab | ||
| if (!this.tabObjects[prevActiveTab]?.disabled) { | ||
| this.internalActiveTabIndex = prevActiveTab; | ||
| this.activeTabIndex = prevActiveTab; |
There was a problem hiding this comment.
should be internalActiveTabIndex.
| @@ -52,38 +67,31 @@ export class IfxTabs { | |||
| // needed for smooth border transition | |||
| reRenderBorder() { | |||
| const borderElement = this.el.shadowRoot.querySelector('.active-border') as HTMLElement; | |||
| if (borderElement && this.tabHeaderRefs[this.activeTabIndex]) { | |||
| if (borderElement && this.tabHeaderRefs[this.internalActiveTabIndex]) { | |||
| if (this.orientation === 'horizontal') { | |||
There was a problem hiding this comment.
I would suggest you add toLowerCase() in case they spell it with a capital letter.
There was a problem hiding this comment.
the orientation? didnt make changes to that and its configured on storybook via 2 options only. But we can add the lowerCase check.
Or do you mean the tabIndex? thats passed as a number, so shouldnt matter
There was a problem hiding this comment.
Yes, I was referring to the orientation prop. I can see people typing Horizontal with capital letter, and wondering why is it not working.
There was a problem hiding this comment.
This one still remains unresolved. You can skip it if you want. It's a minor thing.
|
the keyboard navigation is not working with key navigation anymore, only with tabs. Focus is not working, I told you already |
conuming wrapper components
conuming wrapper components
…thub.com/Infineon/infineon-design-system-stencil into 738-tabs-set-active-tab-from-the-outside
… command in example apps
|
🚀 PR was released in |
By creating this pull request you agree to the terms in CONTRIBUTING.md.
https://github.com/Infineon/.github/blob/master/CONTRIBUTING.md
--- DO NOT DELETE ANYTHING ABOVE THIS LINE ---
CONTRIBUTING.md also tells you what to expect in the PR process.
Description
Tab index can now be set from the outside;
Removed unnecessary code and added the updated Tab component to the Angular+React+Vue example apps
Keyboard navigation added
📦 Published PR as canary version:
20.37.0--canary.741.683712c531c460cfe5ac518ff3c3bae7a5eec2c0.0✨ Test out this PR locally via:
npm install @infineon/infineon-design-system-stencil@20.37.0--canary.741.683712c531c460cfe5ac518ff3c3bae7a5eec2c0.0 # or yarn add @infineon/infineon-design-system-stencil@20.37.0--canary.741.683712c531c460cfe5ac518ff3c3bae7a5eec2c0.0