fix(tab):id attribute added to tab wrapper #224
Conversation
|
There was a problem hiding this comment.
@tatata96 I think it looks good, just I was thinking if we can improve the name like tabContainerId to be more descriptive 🤔
src/tab/Tab.tsx
Outdated
| activeTabIndexFromProps === undefined | ||
| ? activeTabIndex | ||
| : activeTabIndexFromProps |
There was a problem hiding this comment.
The indentation doesn't look correct,
content of [] should be deeper, why was it changed in the first place? 🤔 I believe we already have eslint rules for the toolkit, it should take care of this.
Same for the diff in ControlledTabProps
src/tab/Tab.tsx
Outdated
| // if one of the controlled tab props are present | ||
| // other controlled tab prop is required | ||
| // and initialActiveTabIndex should be undefined | ||
| type ControlledTabProps = | ||
| | { | ||
| activeTabIndex: number; | ||
| onTabChange: (index: number) => void; | ||
| initialActiveTabIndex?: number; | ||
| } | ||
| activeTabIndex: number; | ||
| onTabChange: (index: number) => void; | ||
| initialActiveTabIndex?: number; | ||
| } | ||
| | { | ||
| activeTabIndex?: number; | ||
| onTabChange?: (index: number) => void; | ||
| }; | ||
| activeTabIndex?: number; | ||
| onTabChange?: (index: number) => void; | ||
| }; |
There was a problem hiding this comment.
This is totally unrelated to the issue, but I was trying to understand this comment and the type and I couldn't.
Do you (or anyone) know what is our purpose with this type? Why do we need the second part?
activeTabIndex?: number;
onTabChange?: (index: number) => void;(BTW, code formatting here also looks weird as I mentioned in the other comment)
tatata96
added
Ready for new release
Related Issue;