Skip to content

feat(divider): add support for switching orientation at various breakpoints #7285

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.

Sign up for GitHub

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

Merged
tlabaj merged 5 commits into from
May 2, 2022
Merged

feat(divider): add support for switching orientation at various breakpoints #7285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
901b0f6
add support for switching orientation at various breakpoints
jenny-s51 Apr 20, 2022
a888763
deprecate isVertical, update demos and prop descriptions
jenny-s51 Apr 27, 2022
b1e57d4
PR feedback from mcoker
jenny-s51 Apr 29, 2022
76b24c6
PR feedback from mcoker
jenny-s51 May 2, 2022
78f404d
Merge branch 'main' into iss7225
jenny-s51 May 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion packages/react-core/src/components/Divider/Divider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export interface DividerProps extends React.HTMLProps<HTMLElement> {
className?: string;
/** The component type to use */
component?: 'hr' | 'li' | 'div';
/** Flag to indicate the divider is vertical (must be in a flex layout) */
/** @deprecated Use `orientation` instead. Flag to indicate the divider is vertical (must be in a flex layout) */
isVertical?: boolean;
/** Insets at various breakpoints. */
inset?: {
Expand All @@ -25,13 +25,23 @@ export interface DividerProps extends React.HTMLProps<HTMLElement> {
xl?: 'insetNone' | 'insetXs' | 'insetSm' | 'insetMd' | 'insetLg' | 'insetXl' | 'inset2xl' | 'inset3xl';
'2xl'?: 'insetNone' | 'insetXs' | 'insetSm' | 'insetMd' | 'insetLg' | 'insetXl' | 'inset2xl' | 'inset3xl';
};
/** Indicates how the divider will display at various breakpoints. Vertical divider must be in a flex layout. */
Copy link
Contributor

@mcoker mcoker Apr 29, 2022

Choose a reason for hiding this comment

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

Just a nit and outside the scope, but since we're copying this doc comment, is the bit about it needing to be in a flex layout necessary? I'm assuming so since the vertical divider doesn't really work on its own like the default/horiz variant. If we're going to leave it, technically the parent (or divider itself) just needs a height of some sort, or the parent needs to support align-items so that align-items: stretch on the divider works (this is why it works in flex, but that is supported in grid, too).

Here are a few examples - https://codepen.io/mcoker/pen/ZErzBeg

This is what the vertical divider core docs say

This modifier requires that the parent has an explicit or implicit height, or has a flex or grid based layout parent.

Copy link
Contributor Author

@jenny-s51 jenny-s51 Apr 29, 2022

Choose a reason for hiding this comment

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

Thank you for your helpful comment and clarification here @mcoker ! It looks like the vertical variant only renders in react when it's placed within a Flex layout -- the extra detail in the prop description lets the consumer know they'll need it.

Is setting the height and/or supporting align-items something that can be done in react, or would that need to be done in core? If agreeable with you, I'd be happy to open another issue so we can get this merged 🙂

Copy link
Contributor

@mcoker mcoker Apr 29, 2022

Choose a reason for hiding this comment

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

@jenny-s51 gotcha! I guess it's a little confusing to me since it can be used outside of a CSS flex layout, but mostly works in the <Flex> react layout.

orientation?: {
Copy link
Contributor

@tlabaj tlabaj Apr 26, 2022

Choose a reason for hiding this comment

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

I think we want to add a default here and deprecate the isVertical prop?
@mcoker, thoughts?

Copy link
Contributor

@mcoker mcoker Apr 27, 2022

Choose a reason for hiding this comment

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

@tlabaj yep, that would be inline with how we handle these breakpoint modifiers in other components.

default?: 'vertical' | 'horizontal';
sm?: 'vertical' | 'horizontal';
md?: 'vertical' | 'horizontal';
lg?: 'vertical' | 'horizontal';
xl?: 'vertical' | 'horizontal';
'2xl'?: 'vertical' | 'horizontal';
};
}

export const Divider: React.FunctionComponent<DividerProps> = ({
className,
component = DividerVariant.hr,
isVertical = false,
inset,
orientation,
Copy link
Contributor

@nicolethoen nicolethoen Apr 28, 2022

Choose a reason for hiding this comment

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

can we or should we set the default orientation alongside the other default props? Before, isVertical defaulted to false.

Copy link
Contributor Author

@jenny-s51 jenny-s51 Apr 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Setting the default orientation shouldn't be necessary here since Divider defaults to horizontal! 👍

...props
}: DividerProps) => {
const Component: any = component;
Expand All @@ -42,6 +52,7 @@ export const Divider: React.FunctionComponent<DividerProps> = ({
styles.divider,
isVertical && styles.modifiers.vertical,
formatBreakpointMods(inset, styles),
formatBreakpointMods(orientation, styles),
className
)}
{...(component !== 'hr' && { role: 'separator' })}
Expand Down
6 changes: 5 additions & 1 deletion packages/react-core/src/components/Divider/__tests__/Divider.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ test('vertical divider', () => {
const { asFragment } = render(
<Flex>
<FlexItem>first item</FlexItem>
<Divider isVertical />
<Divider
orientation={{
default: 'vertical'
}}
/>
<FlexItem>second item</FlexItem>
</Flex>
);
Expand Down
6 changes: 6 additions & 0 deletions packages/react-core/src/components/Divider/examples/Divider.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,9 @@ propComponents: ['Divider']

```ts file='./DividerVerticalFlexInsetVariousBreakpoints.tsx'
```

### Switch orientation at various breakpoints

```ts file='./DividerOrientationVariousBreakpoints.tsx'
```

19 changes: 19 additions & 0 deletions packages/react-core/src/components/Divider/examples/DividerOrientationVariousBreakpoints.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React from 'react';
import { Divider, Flex, FlexItem } from '@patternfly/react-core';

export const DividerOrientationVariousBreakpoints: React.FunctionComponent = () => (
<Flex>
<FlexItem>first item</FlexItem>
<Divider
orientation={{
default: 'vertical',
sm: 'horizontal',
md: 'vertical',
lg: 'horizontal',
xl: 'vertical',
'2xl': 'horizontal'
}}
/>
<FlexItem>second item</FlexItem>
</Flex>
);
6 changes: 5 additions & 1 deletion packages/react-core/src/components/Divider/examples/DividerVerticalFlex.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import { Divider, Flex, FlexItem } from '@patternfly/react-core';
export const DividerVerticalFlex: React.FunctionComponent = () => (
<Flex>
<FlexItem>first item</FlexItem>
<Divider isVertical />
<Divider
orientation={{
default: 'vertical'
}}
/>
<FlexItem>second item</FlexItem>
</Flex>
);
7 changes: 6 additions & 1 deletion packages/react-core/src/components/Divider/examples/DividerVerticalFlexInsetMedium.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { Divider, Flex, FlexItem } from '@patternfly/react-core';
export const DividerVerticalFlexInsetMedium: React.FunctionComponent = () => (
<Flex>
<FlexItem>first item</FlexItem>
<Divider isVertical inset={{ default: 'insetMd' }} />
<Divider
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetMd' }}
/>
<FlexItem>second item</FlexItem>
</Flex>
);
6 changes: 4 additions & 2 deletions ...react-core/src/components/Divider/examples/DividerVerticalFlexInsetVariousBreakpoints.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ export const DividerVerticalFlexInsetVariousBreakpoints: React.FunctionComponent
<Flex>
<FlexItem>first item</FlexItem>
<Divider
isVertical
orientation={{
default: 'vertical'
}}
inset={{
default: 'insetMd',
md: 'insetNone',
lg: 'insetSm',
xl: 'insetXs'
}}
/>
<FlexItem>first item</FlexItem>
<FlexItem>second item</FlexItem>
Copy link
Contributor

@mcoker mcoker May 2, 2022

Choose a reason for hiding this comment

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

🤩

</Flex>
);
129 changes: 108 additions & 21 deletions packages/react-core/src/demos/CardDemos.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,14 @@ const AggregateStatusCards: React.FunctionComponent = () => {
<a href="#">{count}</a>
</FlexItem>
</Flex>
{content.length > 1 && index === 0 && <Divider key={`${index}_d`} isVertical />}
{content.length > 1 && index === 0 && (
<Divider
key={`${index}_d`}
orientation={{
default: 'vertical'
}}
/>
)}
</React.Fragment>
))}
</Flex>
Expand Down Expand Up @@ -1255,8 +1262,10 @@ const UtilizationCard3: React.FunctionComponent = () => {

return (
<React.Fragment>
<b>Note:</b> Custom CSS is used in this demo to align the card title and select toggle text to <code>baseline</code> alignment.
<br /><br />
<b>Note:</b> Custom CSS is used in this demo to align the card title and select toggle text to{' '}
<code>baseline</code> alignment.
<br />
<br />
<Gallery hasGutter minWidths={{ default: '360px' }}>
<GalleryItem>
<Card id="utilization-card-1" component="div">
Expand Down Expand Up @@ -1507,7 +1516,13 @@ CardNestedDemo = () => {
<FlexItem>
<b>Temperature</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>64C</span>
</FlexItem>
Expand Down Expand Up @@ -1571,7 +1586,13 @@ CardNestedDemo = () => {
<FlexItem>
<b>Speed</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>2.3Ghz</span>
</FlexItem>
Expand Down Expand Up @@ -1656,7 +1677,13 @@ CardNestedDemo = () => {
<FlexItem>
<b>Temperature</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>64C</span>
</FlexItem>
Expand Down Expand Up @@ -1720,7 +1747,13 @@ CardNestedDemo = () => {
<FlexItem>
<b>Speed</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>2.3Ghz</span>
</FlexItem>
Expand Down Expand Up @@ -1805,7 +1838,13 @@ CardNestedDemo = () => {
<FlexItem>
<b>Temperature</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>64C</span>
</FlexItem>
Expand Down Expand Up @@ -1869,7 +1908,13 @@ CardNestedDemo = () => {
<FlexItem>
<b>Speed</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>2.3Ghz</span>
</FlexItem>
Expand Down Expand Up @@ -1994,7 +2039,13 @@ const AccordionCard: React.FunctionComponent = () => {
<FlexItem>
<b>Temperature</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>64C</span>
</FlexItem>
Expand Down Expand Up @@ -2058,7 +2109,13 @@ const AccordionCard: React.FunctionComponent = () => {
<FlexItem>
<b>Speed</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>2.3Ghz</span>
</FlexItem>
Expand Down Expand Up @@ -2140,7 +2197,13 @@ const AccordionCard: React.FunctionComponent = () => {
<FlexItem>
<b>Temperature</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>64C</span>
</FlexItem>
Expand Down Expand Up @@ -2204,7 +2267,13 @@ const AccordionCard: React.FunctionComponent = () => {
<FlexItem>
<b>Speed</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>2.3Ghz</span>
</FlexItem>
Expand Down Expand Up @@ -2286,7 +2355,13 @@ const AccordionCard: React.FunctionComponent = () => {
<FlexItem>
<b>Temperature</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>64C</span>
</FlexItem>
Expand Down Expand Up @@ -2350,7 +2425,13 @@ const AccordionCard: React.FunctionComponent = () => {
<FlexItem>
<b>Speed</b>
</FlexItem>
<Divider className="pf-u-hidden-on-md" isVertical inset={{ default: 'insetSm' }} />
<Divider
className="pf-u-hidden-on-md"
orientation={{
default: 'vertical'
}}
inset={{ default: 'insetSm' }}
/>
<FlexItem>
<span>2.3Ghz</span>
</FlexItem>
Expand Down Expand Up @@ -2444,8 +2525,10 @@ const TrendCard1: React.FunctionComponent = () => {
];
return (
<React.Fragment>
<b>Note:</b> Custom CSS is used in this demo to align the card title and select toggle text to <code>baseline</code> alignment.
<br /><br />
<b>Note:</b> Custom CSS is used in this demo to align the card title and select toggle text to{' '}
<code>baseline</code> alignment.
<br />
<br />
<Gallery hasGutter minWidths={{ default: '360px' }}>
<GalleryItem>
<Card id="trend-card-1" component="div">
Expand Down Expand Up @@ -2626,8 +2709,10 @@ CardLogViewDemo = () => {

return (
<React.Fragment>
<b>Note:</b> Custom CSS is used in this demo to align the card title and select toggle text to <code>baseline</code> alignment.
<br /><br />
<b>Note:</b> Custom CSS is used in this demo to align the card title and select toggle text to{' '}
<code>baseline</code> alignment.
<br />
<br />
<Gallery hasGutter style={{ '--pf-l-gallery--GridTemplateColumns--min': '360px' }}>
<Card id="card-log-view-example">
<CardHeader className="pf-u-align-items-flex-start">
Expand Down Expand Up @@ -2743,8 +2828,10 @@ CardEventViewDemo = () => {

return (
<React.Fragment>
<b>Note:</b> Custom CSS is used in this demo to align the card title and select toggle text to <code>baseline</code> alignment.
<br /><br />
<b>Note:</b> Custom CSS is used in this demo to align the card title and select toggle text to{' '}
<code>baseline</code> alignment.
<br />
<br />
<Gallery hasGutter style={{ '--pf-l-gallery--GridTemplateColumns--min': '360px' }}>
<Card id="card-events-view-example">
<CardHeader className="pf-u-align-items-flex-start">
Expand Down