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

[Stack] Add useFlexGap prop #36404

Merged
merged 8 commits into from
Mar 27, 2023
Merged

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Mar 3, 2023

@siriwatknp siriwatknp added new feature New feature or request package: system Specific to @mui/system package: material-ui Specific to @mui/material package: joy-ui Specific to @mui/joy labels Mar 3, 2023
@siriwatknp siriwatknp requested a review from a team March 3, 2023 06:08
@mui-bot
Copy link

mui-bot commented Mar 3, 2023

@siriwatknp siriwatknp changed the title [system] Add useFlexGap prop [Stack] Add useFlexGap prop Mar 3, 2023
@oliviertassinari oliviertassinari added the component: Stack The React component. label Mar 3, 2023

## Flexbox gap

To use [flexbox `gap`](https://developer.mozilla.org/en-US/docs/Web/CSS/gap) for the spacing implementation, set flag `useFlexGap` to true.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To use [flexbox `gap`](https://developer.mozilla.org/en-US/docs/Web/CSS/gap) for the spacing implementation, set flag `useFlexGap` to true.
To use [flexbox `gap`](https://developer.mozilla.org/en-US/docs/Web/CSS/gap) for the spacing implementation, set the prop `useFlexGap` to true.

x all the other instances, I doubt we should ever use "flag" in this context.

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

niice

Comment on lines +43 to +45
<Stack>
<Avatar>W</Avatar>
</Stack>
Copy link
Member

Choose a reason for hiding this comment

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

Is this a typo?

Suggested change
<Stack>
<Avatar>W</Avatar>
</Stack>
<Avatar>W</Avatar>

it looks wrong here too https://mui.com/joy-ui/react-stack/#white-space-nowrap in prod.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so but let's fix it in another PR. It is not related to the useFlexGap prop.


To use [flexbox `gap`](https://developer.mozilla.org/en-US/docs/Web/CSS/gap) for the spacing implementation, set flag `useFlexGap` to true.

It removes the [known limitation](#limitations) of the default implementation that uses CSS nested selector. However, CSS flexbox gap is not fully supported in some browsers. We recommend to check the [support percentage](https://caniuse.com/?search=flex%20gap) before using it.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
It removes the [known limitation](#limitations) of the default implementation that uses CSS nested selector. However, CSS flexbox gap is not fully supported in some browsers. We recommend to check the [support percentage](https://caniuse.com/?search=flex%20gap) before using it.
It removes the [known limitation](#limitations) of the default implementation that uses CSS nested selector. However, CSS flexbox gap is not fully supported in some browsers.
We recommend checking the [support percentage](https://caniuse.com/?search=flex%20gap) before using it.

x other instances

}));

const message = `Truncation should be conditionally applicable on this long line of text
as this is a much longer line than what the container can support. `;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
as this is a much longer line than what the container can support. `;
as this is a much longer line than what the container can support.`;

@@ -24,6 +24,14 @@ export interface StackBaseProps {
* Add an element between each child.
*/
divider?: React.ReactNode;
/**
* If `true`, the CSS flex `gap` is used instead of the pseudo selector approach.
* To enable this flag globally, follow the theme's default props configuration.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* To enable this flag globally, follow the theme's default props configuration.
* To enable this prop globally, follow the theme's default props configuration.

* If `true`, the CSS flex `gap` is used instead of the pseudo selector approach.
* To enable this flag globally, follow the theme's default props configuration.
*
* ⚠️ Warning: CSS flex `gap` is not fully supported in some browsers, we recommend to check https://caniuse.com/?search=flex%20gap before using this flag.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* ⚠️ Warning: CSS flex `gap` is not fully supported in some browsers, we recommend to check https://caniuse.com/?search=flex%20gap before using this flag.
* ⚠️ Warning: CSS flex `gap` is not fully supported in some browsers, we recommend checking https://caniuse.com/?search=flex%20gap before using this flag.

packages/mui-joy/src/Stack/StackProps.ts Outdated Show resolved Hide resolved
packages/mui-joy/src/Stack/StackProps.ts Outdated Show resolved Hide resolved
packages/mui-material/src/Stack/Stack.d.ts Outdated Show resolved Hide resolved
packages/mui-material/src/Stack/Stack.d.ts Outdated Show resolved Hide resolved
@siriwatknp siriwatknp requested a review from hbjORbj March 20, 2023 04:46
@siriwatknp
Copy link
Member Author

@hbjORbj Can you take a look at the description again?

packages/mui-system/src/Stack/StackProps.ts Outdated Show resolved Hide resolved
packages/mui-system/src/Stack/StackProps.ts Outdated Show resolved Hide resolved
@siriwatknp siriwatknp requested review from hbjORbj and mnajdova March 22, 2023 04:38
Copy link
Member

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

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

Looks good👍👍

@siriwatknp siriwatknp merged commit 8a6e1a7 into mui:master Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Stack The React component. new feature New feature or request package: joy-ui Specific to @mui/joy package: material-ui Specific to @mui/material package: system Specific to @mui/system
Projects
None yet
4 participants