Fix handling of style property in Tabs component#139
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #139 +/- ##
=======================================
Coverage 92.22% 92.22%
=======================================
Files 87 87
Lines 1504 1505 +1
Branches 231 230 -1
=======================================
+ Hits 1387 1388 +1
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:
|
clarasb
requested review from
b-yogesh and
forman
and removed request for
b-yogesh
b-yogesh
left a comment
b-yogesh
left a comment
There was a problem hiding this comment.
Thanks. Looks good. Please have a look at my comments.
| type: "Tab"; | ||
| label?: string; | ||
| icon?: string; | ||
| iconPosition?: "bottom" | "end" | "start" | "top" | undefined; |
There was a problem hiding this comment.
why do we have undefined here?
Also, in the API docs https://mui.com/material-ui/api/tab/, the default is set to top. If you change that to undefined, does it affect it in any way?
There was a problem hiding this comment.
You're right, undefined shouldn't be here. I removed it.
| return ( | ||
| <MuiTab | ||
| key={index} | ||
| style={tabState?.style} |
There was a problem hiding this comment.
we apply the same style to both the Tab button and body, was this a design decision? or does it needs discussion?
There was a problem hiding this comment.
Correct, this enables a user to apply styling to the entire Tab component (Button in the header + body).
Before this change it was only possible to change the styling of the button and the user had no styling control over the content (body) of the Tab (see #135).
However, this desicion was not discussed.
There was a problem hiding this comment.
@b-yogesh is right, using the same style for both cannot be right.
There was a problem hiding this comment.
I applied your change requests. Now, the style of the Tab component gets applied only to the Tab button, the styling of the Tab content needs to be managed by the children of the Tab component.
forman
left a comment
forman
left a comment
There was a problem hiding this comment.
Just the style thingy, otherwise LGTM.
| return ( | ||
| <MuiTab | ||
| key={index} | ||
| style={tabState?.style} |
There was a problem hiding this comment.
@b-yogesh is right, using the same style for both cannot be right.
3 participants
This PR:
styleprop inTabscomponent.iconPositiontoTabscomponentExample Panel H (chartlets.py/demo/my_extension/my_panel_8.py):
Closes #135 and #136.