-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[docs] Docs redesign adjustments #28203
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
const DrawerPaper = styled(Paper)({ | ||
width: 360, | ||
borderTopRightRadius: 0, | ||
borderBottomRightRadius: 0, | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mnajdova we might need to investigate more on this, it is very buggy and hard to reason. by using styled component and pass to <Drawer PaperProps={{ component: DrawerPaper, square: true, elevation: 0 }}
, it is still rounded
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will take a look thanks for pointing it out. I think that the general problem is that the portaled components don't support the components
& componentsProps
which would replace the components underneath instead of aliasing it to the as
property (which is what we currently do).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently what you have is: <StyledPaper as={AnotherStyledPaper} />
- which results in double classes and non-deterministic classes order in the head.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably just set the sx
prop :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, we need to have some story around this in the migration guide I think. I agree it is hard to reason about.
In v4 when you provide a component
it was changing the element that is rendered, but it also was adding the classes
for that element, so the styles were applied.
In v5, to make it work identical, we are mapping the component
prop to the as
prop, so that the styles defined originally in the component would still be applied. This however does not guarantees that the styles for the component defined via the as
property would win.
I think that we should specify in the migration guide, that for cases like this, when SomeComponentProps
is used, we recommend sx
for overriding the styles. When we add the components
& componentsProps
API on all components, that would add a possibility for completely replacing that slot with a new component. Note that the new component can actually be a styled version of the original component, and in that case we would be able to actually override the styles for it.
Does this summary makes sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a sandbox illustrating the current behavior https://codesandbox.io/s/magical-mahavira-6p39p?file=/src/App.js
This comment has been minimized.
This comment has been minimized.
Is reducing font-size helps? @danilo-leal |
Way better! Thank you, Jun! |
Mm... Not so much I guess. I think it's more about the font-weight than anything. Ideally, we should be able to set different font-weights depending on the application of the inline code (heading or paragraph). Is it feasible? |
@danilo-leal so font-weight bold for inline code in the heading? |
We could try that, can you make an iteration of it to see how it goes? |
This comment has been minimized.
This comment has been minimized.
Iterated a bit more, reducing just a tad the font size and standardizing the color. I think we're good to do with this one! |
Some adjustments in mind (mostly minor stuff):
Edit this page
button: it is currently absolute positioned. Maybe it's best to have it in a flex container together with the title and description of the page for a safer responsive behavior. Here's an example of what I described.Templates
andBlog
links, on the Nav Drawer, with the same styles as all the others: box with an icon + arrow. On the example below, I'm using theBookOutlined
andChromeReaderModeOutlined
icons. Also, make the typography variant the same and with the dividing lines.