Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #129 +/- ##
==========================================
+ Coverage 92.17% 92.21% +0.03%
==========================================
Files 86 86
Lines 1496 1503 +7
Branches 225 231 +6
==========================================
+ Hits 1379 1386 +7
Misses 98 98
Partials 19 19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…panel_8 # Conflicts: # chartlets.js/CHANGES.md
b-yogesh
left a comment
b-yogesh
left a comment
There was a problem hiding this comment.
The content inside the tabs dont show for me, am I missing something?
|
@b-yogesh: Did you run |
Yes, I forgot that, it works fine now. |
b-yogesh
left a comment
b-yogesh
left a comment
There was a problem hiding this comment.
It works. I added a couple of Box components following the MUI examples shown here: https://mui.com/material-ui/react-tabs/#BasicTabs.tsx.
Although it looks like the suggested change is quite a lot, it is just a couple of lines. If you agree, please incorporate these changes.
forman
left a comment
forman
left a comment
There was a problem hiding this comment.
I don't want to block merging, but the code readability can be improved (see my comments). Maybe later.
Otherwise good, so I approve!
| <MuiBox sx={{ borderBottom: 1, borderColor: "divider" }}> | ||
| <MuiTabs id={id} style={style} value={value} onChange={handleChange}> | ||
| {tabItems?.map((tab, index) => { | ||
| const tabState = isComponentState(tab) |
There was a problem hiding this comment.
You can make this cleaner code if you introduce a type guard isTabState().
| } | ||
| disabled={disabled || (tabState && tabState.disabled)} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
You can make this cleaner by constructing separate MuiTab components, one for the case where !tabState and the other where you have it.
Even better would be to ensure / assert upfront whether tabItems contains only TabState instances.
3 participants
This PR:
Tabscomponent.Tabscomponent