Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class CatalogTile extends React.Component<CatalogTileProps> {
href={href || '#'}
className={css('catalog-tile-pf', { featured }, className)}
onClick={e => this.handleClick(e)}
isHoverable
isSelectable
{...props}
>
{(badges.length > 0 || iconImg || iconClass || icon) && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,18 @@ exports[`CatalogTile href renders properly 1`] = `
component="a"
href="http://patternfly.org"
id="test-href"
isHoverable={true}
isSelectable={true}
onClick={[Function]}
>
<a
className="pf-c-card pf-m-hoverable catalog-tile-pf featured"
className="pf-c-card pf-m-selectable catalog-tile-pf featured"
data-ouia-component-id="OUIA-Generated-Card-6"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
href="http://patternfly.org"
id="test-href"
onClick={[Function]}
tabIndex="0"
>
<CardTitle
className="catalog-tile-pf-header"
Expand Down Expand Up @@ -128,17 +129,18 @@ exports[`CatalogTile renders properly 1`] = `
component="div"
href="#"
id="single-badge-test"
isHoverable={true}
isSelectable={true}
onClick={[Function]}
>
<div
className="pf-c-card pf-m-hoverable catalog-tile-pf featured"
className="pf-c-card pf-m-selectable catalog-tile-pf featured"
data-ouia-component-id="OUIA-Generated-Card-1"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
href="#"
id="single-badge-test"
onClick={[Function]}
tabIndex="0"
>
<CardHeader>
<div
Expand Down Expand Up @@ -407,17 +409,18 @@ exports[`CatalogTile renders properly 1`] = `
component="div"
href="#"
id="multi-badge-test"
isHoverable={true}
isSelectable={true}
onClick={[Function]}
>
<div
className="pf-c-card pf-m-hoverable catalog-tile-pf"
className="pf-c-card pf-m-selectable catalog-tile-pf"
data-ouia-component-id="OUIA-Generated-Card-2"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
href="#"
id="multi-badge-test"
onClick={[Function]}
tabIndex="0"
>
<CardHeader>
<div
Expand Down Expand Up @@ -757,17 +760,18 @@ exports[`CatalogTile renders properly 1`] = `
component="div"
href="#"
id="test-iconClass"
isHoverable={true}
isSelectable={true}
onClick={[Function]}
>
<div
className="pf-c-card pf-m-hoverable catalog-tile-pf"
className="pf-c-card pf-m-selectable catalog-tile-pf"
data-ouia-component-id="OUIA-Generated-Card-3"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
href="#"
id="test-iconClass"
onClick={[Function]}
tabIndex="0"
>
<CardHeader>
<div
Expand Down Expand Up @@ -994,17 +998,18 @@ exports[`CatalogTile renders properly 1`] = `
component="a"
href="https://github.com/patternfly/patternfly-react"
id="tile-footer-test"
isHoverable={true}
isSelectable={true}
onClick={[Function]}
>
<a
className="pf-c-card pf-m-hoverable catalog-tile-pf featured"
className="pf-c-card pf-m-selectable catalog-tile-pf featured"
data-ouia-component-id="OUIA-Generated-Card-4"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
href="https://github.com/patternfly/patternfly-react"
id="tile-footer-test"
onClick={[Function]}
tabIndex="0"
>
<CardHeader>
<div
Expand Down Expand Up @@ -1255,17 +1260,18 @@ exports[`CatalogTile renders properly 1`] = `
component="a"
href="https://github.com/patternfly/patternfly-react"
id="custom-icon-svg-test"
isHoverable={true}
isSelectable={true}
onClick={[Function]}
>
<a
className="pf-c-card pf-m-hoverable catalog-tile-pf featured"
className="pf-c-card pf-m-selectable catalog-tile-pf featured"
data-ouia-component-id="OUIA-Generated-Card-5"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
href="https://github.com/patternfly/patternfly-react"
id="custom-icon-svg-test"
onClick={[Function]}
tabIndex="0"
>
<CardHeader>
<div
Expand Down
40 changes: 23 additions & 17 deletions packages/react-core/src/components/Card/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,18 @@ export interface CardProps extends React.HTMLProps<HTMLElement>, OUIAProps {
className?: string;
/** Sets the base component to render. defaults to article */
component?: keyof JSX.IntrinsicElements;
/** Modifies the card to include hover styles on :hover */
/** @deprecated to make a card hoverable, use isSelectable or isSelectableRaised. */
isHoverable?: boolean;
/** @beta Modifies the card to include hoverable-raised styles on :hover, this styling is included by default with isSelectableRaised */
isHoverableRaised?: boolean;
/** Modifies the card to include compact styling. Should not be used with isLarge. */
isCompact?: boolean;
/** Modifies the card to include selectable styling */
isSelectable?: boolean;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

so, is this prop deprecated?

Copy link
Copy Markdown
Contributor

@kmcfaul kmcfaul Dec 2, 2021

Choose a reason for hiding this comment

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

Are we going to keep supporting the original plain variation of selectable cards? If not I'd say the next breaking release we update isSelectable to use the raised styling and remove isSelectableRaised (I'm just not sure if that really falls under the "deprecated" flag indication though). If we will keep the original styling I think we should bring back in the enum for the style variations and keep using isSelectable as is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes. I was confused at first. I thought the original variant prop also set the card to be selectable.

/** @beta Specifies the card is selectable, and applies the new raised styling on hover and select */
isSelectableRaised?: boolean;
/** Modifies the card to include selected styling */
isSelected?: boolean;
/** @beta Modifies the card to include selectable-raised styling and hoverable-raised styling */
isSelectableRaised?: boolean;
/** @beta Modifies the card to include selected-raised styling */
isSelectedRaised?: boolean;
/** @beta Modifies a raised selectable card to have disabled styling */
isDisabledRaised?: boolean;
/** Modifies the card to include flat styling */
isFlat?: boolean;
/** Modifies the card to include rounded styling */
Expand Down Expand Up @@ -56,12 +54,11 @@ export const Card: React.FunctionComponent<CardProps> = ({
className = '',
component = 'article',
isHoverable = false,
isHoverableRaised = false,
isCompact = false,
isSelectable = false,
isSelected = false,
isSelectableRaised = false,
isSelectedRaised = false,
isSelected = false,
isDisabledRaised = false,
isFlat = false,
isExpanded = false,
isRounded = false,
Expand All @@ -79,6 +76,20 @@ export const Card: React.FunctionComponent<CardProps> = ({
console.warn('Card: Cannot use isCompact with isLarge. Defaulting to isCompact');
isLarge = false;
}

const getSelectableModifiers = () => {
if (isDisabledRaised) {
return css(styles.modifiers.nonSelectableRaised);
}
if (isSelectableRaised) {
return css(styles.modifiers.selectableRaised, isSelected && styles.modifiers.selectedRaised);
}
if (isSelectable || isHoverable) {
return css(styles.modifiers.selectable, isSelected && styles.modifiers.selected);
}
return '';
};

return (
<CardContext.Provider
value={{
Expand All @@ -90,22 +101,17 @@ export const Card: React.FunctionComponent<CardProps> = ({
id={id}
className={css(
styles.card,
isHoverable && styles.modifiers.hoverable,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mcoker, could someone apply pf-m-hoverable. Without, isSelectable. I think removing support completely would technically be breaking. I think we can deprecate it, but we still need to apply the style.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tlabaj yep, they're separate. While you could use them together, it isn't necessary since isSelected has its own hoverable styles (they're the same as what isHoverable applies). https://github.com/patternfly/patternfly/blob/dc6cf07d69cbb80026dca7fe43dc28704aabea64/src/patternfly/components/Card/card.scss#L26-L27

isHoverableRaised && styles.modifiers.hoverableRaised,
isCompact && styles.modifiers.compact,
isSelectable && !isSelectableRaised && styles.modifiers.selectable,
isSelected && !isSelectedRaised && isSelectable && styles.modifiers.selected,
isSelectableRaised && styles.modifiers.selectableRaised,
isSelectedRaised && isSelectableRaised && styles.modifiers.selectedRaised,
isExpanded && styles.modifiers.expanded,
isFlat && styles.modifiers.flat,
isRounded && styles.modifiers.rounded,
isLarge && styles.modifiers.displayLg,
isFullHeight && styles.modifiers.fullHeight,
isPlain && styles.modifiers.plain,
getSelectableModifiers(),
className
)}
tabIndex={isSelectableRaised || isSelectable ? '0' : undefined}
tabIndex={isSelectable || isSelectableRaised ? '0' : undefined}
{...props}
{...ouiaProps}
>
Expand Down
22 changes: 8 additions & 14 deletions packages/react-core/src/components/Card/__tests__/Card.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ test('card with isHoverable applied ', () => {
expect(view).toMatchSnapshot();
});

test('card with isHoverableRaised applied ', () => {
const view = shallow(<Card isHoverableRaised />).dive();
expect(view).toMatchSnapshot();
});

test('card with isCompact applied ', () => {
const view = shallow(<Card isCompact />).dive();
expect(view).toMatchSnapshot();
Expand All @@ -64,25 +59,24 @@ test('card with only isSelected applied - not change', () => {
expect(view.prop('tabIndex')).toBe(undefined);
});

test('card with isSelectableRaised applied ', () => {
test('card with isDisabledRaised applied', () => {
const view = shallow(<Card isDisabledRaised />).dive();
expect(view.prop('className')).toMatch(/non-selectable-raised/);
});

test('card with isSelectableRaised applied - not change', () => {
const view = shallow(<Card isSelectableRaised />).dive();
expect(view.prop('className')).toMatch(/selectable-raised/);
expect(view.prop('tabIndex')).toBe('0');
});

test('card with isSelectableRaised and isSelectedRaised applied ', () => {
const view = shallow(<Card isSelectableRaised isSelectedRaised />).dive();
test('card with isSelectableRaised and isSelected applied ', () => {
const view = shallow(<Card isSelected isSelectableRaised />).dive();
expect(view.prop('className')).toMatch(/selectable-raised/);
expect(view.prop('className')).toMatch(/selected-raised/);
expect(view.prop('tabIndex')).toBe('0');
});

test('card with only isSelectedRaised applied - not change', () => {
const view = shallow(<Card isSelectedRaised />).dive();
expect(view.prop('className')).not.toMatch(/selected-raised/);
expect(view.prop('tabIndex')).toBe(undefined);
});

test('card with isFlat applied', () => {
const view = shallow(<Card isFlat />).dive();
expect(view.prop('className')).toMatch(/m-flat/);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`card with isCompact applied 1`] = `
<article
className="pf-c-card pf-m-compact"
data-ouia-component-id="OUIA-Generated-Card-8"
data-ouia-component-id="OUIA-Generated-Card-7"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
id=""
Expand All @@ -12,24 +12,14 @@ exports[`card with isCompact applied 1`] = `

exports[`card with isHoverable applied 1`] = `
<article
className="pf-c-card pf-m-hoverable"
className="pf-c-card pf-m-selectable"
data-ouia-component-id="OUIA-Generated-Card-6"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
id=""
/>
`;

exports[`card with isHoverableRaised applied 1`] = `
<article
className="pf-c-card pf-m-hoverable-raised"
data-ouia-component-id="OUIA-Generated-Card-7"
data-ouia-component-type="PF4/Card"
data-ouia-safe={true}
id=""
/>
`;

exports[`className is added to the root element 1`] = `"pf-c-card extra-class"`;

exports[`renders with PatternFly Core styles 1`] = `
Expand Down
Loading