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

fix(Card): fix toggle-in-card navigation #165

Merged
merged 9 commits into from
Nov 9, 2022
2 changes: 1 addition & 1 deletion .storybook/preview-head.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<link
href="https://assets.finn.no/pkg/@fabric-ds/css/v0/fabric.min.css"
href="https://assets.finn.no/pkg/@fabric-ds/css/v1/fabric.min.css"
rel="stylesheet"
/>
2 changes: 1 addition & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
}
</style>
<link
href="https://assets.finn.no/pkg/@fabric-ds/css/v0/fabric.min.css"
href="https://assets.finn.no/pkg/@fabric-ds/css/v1/fabric.min.css"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, we should really find a way to base this off the version of Fabric CSS listed in package.json. Job for another time though probably.

rel="stylesheet"
/>
<link
Expand Down
7 changes: 3 additions & 4 deletions packages/_helpers/clickable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export type ClickableProps = {

/**
* Click handler
* If passed, clickable renders as a button with a click event
*/
onClick?: () => void;
} & Partial<Omit<HTMLAnchorElement, 'children'>> &
Expand All @@ -56,12 +55,12 @@ export function Clickable({
return radio || checkbox ? (
<ToggleItem
labelClassName={props.labelClassName}
className="absolute inset-0 h-full w-full appearance-none cursor-pointer focus-ring"
className="focus-ring focus-ring-inset absolute inset-0 h-full w-full appearance-none cursor-pointer"
type={type}
controlled={false}
onChange={() => undefined}
onChange={props.onClick ? props.onClick : () => undefined}
value={value}
name={`${id}:toggle`}
name={`${props.name || id}:toggle`}
>
{children}
</ToggleItem>
Expand Down
2 changes: 1 addition & 1 deletion packages/_helpers/dead-toggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function DeadToggle(props: DeadToggleProps) {
type={type}
className="hidden"
labelClassName={props.labelClassName}
name="test"
name="dead-toggle"
controlled={true}
onChange={() => undefined}
value={props.value}
Expand Down
116 changes: 62 additions & 54 deletions packages/card/docs/Card.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ function Example() {

return (
<div className="space-y-32 md:space-y-0 md:grid grid-cols-3 gap-32">
<Card onClick={() => setSelected(!selected)} selected={selected}>
<Card selected={selected}>
<img
className="h-128 w-full object-cover"
src="https://source.unsplash.com/random/400x400"
Expand All @@ -184,8 +184,10 @@ function Example() {
<div className="p-16">
<p className="text-12 text-gray-300">DNB Eiendom</p>
<p>
Stilfull og gjennomgående 3-roms m/balkong. Oppusset i 2019. Inkl.
bl.a. vv/fyring.
<Clickable checkbox onClick={() => setSelected(!selected)}>
Stilfull og gjennomgående 3-roms m/balkong. Oppusset i 2019. Inkl.
bl.a. vv/fyring.
</Clickable>
</p>
<p className="text-14 text-gray-400 mb-4">Bøgata 25C, 0655 Oslo</p>
<p className="font-bold my-8">
Expand All @@ -207,7 +209,7 @@ function Example() {
</p>
</div>
</Card>
<Card onClick={() => setSelected(!selected)} selected={selected}>
<Card selected={selected}>
<img
className="h-128 w-full object-cover"
src="https://source.unsplash.com/random/403x403"
Expand All @@ -216,8 +218,10 @@ function Example() {
<div className="p-16">
<p className="text-12 text-gray-300">DNB Eiendom</p>
<p>
Stilfull og gjennomgående 3-roms m/balkong. Oppusset i 2019. Inkl.
bl.a. vv/fyring.
<Clickable checkbox onClick={() => setSelected(!selected)}>
Stilfull og gjennomgående 3-roms m/balkong. Oppusset i 2019. Inkl.
bl.a. vv/fyring.
</Clickable>
</p>
<p className="text-14 text-gray-400 mb-4">Bøgata 25C, 0655 Oslo</p>
<p className="font-bold my-8">
Expand All @@ -239,7 +243,7 @@ function Example() {
</p>
</div>
</Card>
<Card onClick={() => setSelected(!selected)} selected={selected}>
<Card selected={selected}>
<img
className="h-128 w-full object-cover"
src="https://source.unsplash.com/random/404x404"
Expand All @@ -248,8 +252,10 @@ function Example() {
<div className="p-16">
<p className="text-12 text-gray-300">DNB Eiendom</p>
<p>
Stilfull og gjennomgående 3-roms m/balkong. Oppusset i 2019. Inkl.
bl.a. vv/fyring.
<Clickable checkbox onClick={() => setSelected(!selected)}>
Stilfull og gjennomgående 3-roms m/balkong. Oppusset i 2019. Inkl.
bl.a. vv/fyring.
</Clickable>
</p>
<p className="text-14 text-gray-400 mb-4">Bøgata 25C, 0655 Oslo</p>
<p className="font-bold my-8">
Expand Down Expand Up @@ -360,35 +366,39 @@ function Example() {

return (
<div>
<Card
selected={checked}
onClick={() => setChecked(!checked)}
className="mt-32 w-max"
>
<div className="p-24 flex items-center">
<DeadToggle checkbox checked={checked} className="-mt-8" />
<Clickable checkbox labelClassName="ml-12">
Checkbox in a card
</Clickable>
</div>
<Card selected={checked} className="mt-32 w-max p-24 flex items-center">
<DeadToggle checkbox checked={checked} className="-mt-8" />
<Clickable
checkbox
labelClassName="ml-12"
onClick={() => setChecked(!checked)}
>
Checkbox in a card
</Clickable>
</Card>

<div className="flex gap-32 mt-32">
<Card selected={selected === 'a'} onClick={() => setSelected('a')}>
<div className="p-24 flex items-center">
<DeadToggle radio checked={selected === 'a'} className="-mt-8" />
<Clickable radio labelClassName="ml-12">
Radio in a card - A
</Clickable>
</div>
<Card selected={selected === 'a'} className="p-24 flex items-center">
<DeadToggle radio checked={selected === 'a'} className="-mt-8" />
<Clickable
radio
labelClassName="ml-12"
name="gfdhjk2"
onClick={() => setSelected('a')}
>
Radio in a card - A
</Clickable>
</Card>
<Card selected={selected === 'b'} onClick={() => setSelected('b')}>
<div className="p-24 flex items-center">
<DeadToggle radio checked={selected === 'b'} className="-mt-8" />
<Clickable radio labelClassName="ml-12">
Radio in a card - B
</Clickable>
</div>
<Card selected={selected === 'b'} className="p-24 flex items-center">
<DeadToggle radio checked={selected === 'b'} className="-mt-8" />
<Clickable
radio
labelClassName="ml-12"
name="gfdhjk2"
onClick={() => setSelected('b')}
>
Radio in a card - B
</Clickable>
</Card>
</div>
</div>
Expand All @@ -415,34 +425,32 @@ function Example() {
<div className="flex">
<Card
flat
className="py-12 px-16 w-max"
className="py-12 px-16 w-max flex items-center"
selected={selected === 'a'}
onClick={() => setSelected('a')}
>
<div className="flex items-center">
<DeadToggle radio checked={selected === 'a'} className="-mt-6" />
<div className="ml-16">
<h4 className="mb-0">
<Clickable radio>Purchase foo</Clickable>
</h4>
<p className="mb-0 text-14">520 kr/mnd</p>
</div>
<DeadToggle radio checked={selected === 'a'} className="-mt-6" />
<div className="ml-16">
<h4 className="mb-0">
<Clickable radio name="purchase" onClick={() => setSelected('a')}>
Purchase foo
</Clickable>
</h4>
<p className="mb-0 text-14">520 kr/mnd</p>
</div>
</Card>
<Card
flat
className="py-12 px-16 w-max ml-20"
className="py-12 px-16 w-max ml-20 flex items-center"
selected={selected === 'b'}
onClick={() => setSelected('b')}
>
<div className="flex items-center">
<DeadToggle radio checked={selected === 'b'} className="-mt-6" />
<div className="ml-16">
<h4 className="mb-0">
<Clickable radio>Purchase bar</Clickable>
</h4>
<p className="mb-0 text-14">124 kr/mnd</p>
</div>
<DeadToggle radio checked={selected === 'b'} className="-mt-6" />
<div className="ml-16">
<h4 className="mb-0">
<Clickable radio name="purchase" onClick={() => setSelected('b')}>
Purchase bar
</Clickable>
</h4>
<p className="mb-0 text-14">124 kr/mnd</p>
</div>
</Card>
</div>
Expand Down
27 changes: 17 additions & 10 deletions packages/card/src/component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,17 @@ import React from 'react';
import { card as c } from '@fabric-ds/css/component-classes';
import { classNames } from '@chbphone55/classnames';
import { CardProps } from './props';
import { useLogDeprecationWarning } from '../../utils/src';

export function Card(props: CardProps) {
const { as = 'div', children, flat, ...rest } = props;

useLogDeprecationWarning({
condition: !!props.onClick,
message:
"'onClick' prop in Card is deprecated. Use Clickable component to handle click events in Cards.",
});

return React.createElement(
as,
{
Expand All @@ -17,6 +25,7 @@ export function Card(props: CardProps) {
[props.selected ? c.cardFlatSelected : c.cardFlatUnselected]:
props.flat,
}),
// @balbinak(08.11.22): onClick support in Card is deprecated. Remove when Fabric React users are ready for this major change
BalbinaK marked this conversation as resolved.
Show resolved Hide resolved
tabIndex: props.onClick ? 0 : undefined,
onKeyDown: props.onClick
? (e) => {
Expand All @@ -43,18 +52,16 @@ export function Card(props: CardProps) {
</button>
)}

{!props.onClick && props.selected && (
<span role="checkbox" aria-checked="true" aria-disabled="true" />
{!props.flat && (
<div
className={classNames({
[c.cardOutline]: true,
[props.selected ? c.cardOutlineSelected : c.cardOutlineUnselected]:
true,
})}
/>
)}

<div
className={classNames({
[c.cardOutline]: !props.flat,
[props.selected ? c.cardOutlineSelected : c.cardOutlineUnselected]:
!props.flat,
})}
/>

{children}
</>,
);
Expand Down
2 changes: 1 addition & 1 deletion packages/card/src/props.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface CardProps {
className?: string;

/**
* When the card is clicked
* When the card is clicked (deprecated)
*/
onClick?: () => void;
}
Loading