Skip to content
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] Add missing justifyContent values and update box styling #29117

Merged
merged 5 commits into from Oct 20, 2021

Conversation

omarmosid
Copy link
Contributor

Closes: #29050

image

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 17, 2021

No bundle size changes

Generated by 🚫 dangerJS against 3c10e4b

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me except the styles of the Item in the demo. Can you make it consistent with other demos? (with bg grey and black text)

Screen Shot 2564-10-18 at 23 26 47

@omarmosid
Copy link
Contributor Author

@siriwatknp Sure I can do that but I made the change because @oliviertassinari mentioned it in the original issue #29050

image

It would perhaps make sense to go ahead and make all the boxes on the flexbox page look like how the boxes on the grid page look. I can make this change as part of this PR (if you're okay with it) or just revert back to the gray ones for now.

Will wait for your confirmation.

@siriwatknp
Copy link
Member

@omarmosid Thanks for pointing that out. I think all of the boxes in grey can be converted to blue (similar to grid demos) in this PR! please go ahead with blue boxes.

BTW, Nice work!

…, alignSelf, display, flexDirection, flexGrow, flexShrink, flexWrap, and Order and update box item styling
@omarmosid
Copy link
Contributor Author

omarmosid commented Oct 19, 2021

Thankyou for the feedback @siriwatknp.

I have now updated the PR and added a few missing examples for css properties that can be used for flexDirection, alignItems, alignContent, alignSelf, display, flexDirection, flexGrow, flexShrink, flexWrap, and Order

I have also taken the liberty to add links to the relevent MDN documentation under each property title. Hope that's going to be okay. It's worth noting here that these links open in the same page since they are your normal markdown links and we may want to have them open in a new tab. So I could change:

For more information please see [flex-direction](https://developer.mozilla.org/en-US/docs/Web/CSS/flex-direction) on MDN.

with

For more information please see 
<a href="https://developer.mozilla.org/en-US/docs/Web/CSS/flex-direction" target="_blank" rel="noopener noreferrer">flex-direction</a> 
on MDN.

Again will wait for your confirmation, before making this change.

@siriwatknp
Copy link
Member

Again will wait for your confirmation, before making this change.

Sounds good to me, please go ahead.

@siriwatknp
Copy link
Member

@omarmosid If you don't mind, I would like to reduce the font-size from 19px to 16px.

@omarmosid
Copy link
Contributor Author

@omarmosid If you don't mind, I would like to reduce the font-size from 19px to 16px.

No problem at all. Please feel free to let me know in case any more changes are needed 😄

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice work. Thanks for your first contribution and welcome to MUI community!

@siriwatknp siriwatknp changed the title Add missing documentation (examples) for justifyContent and update item box styling [docs] Add missing justifyContent values and update box styling Oct 20, 2021
@siriwatknp siriwatknp merged commit b7d83ae into mui:master Oct 20, 2021
@zannager zannager added the docs Improvements or additions to the documentation label Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[system] Missing documentation in flex-box
4 participants