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

feat(apps/studio): Checkbox implementation for Storybook #688

Merged
merged 9 commits into from
Sep 1, 2023
165 changes: 145 additions & 20 deletions apps/studio/stories/Checkbox.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,34 +1,159 @@
import { Meta, StoryObj } from "@storybook/react";

import { Checkbox, Label } from "@kampus/ui";

const labels = ["köpekleri çıkarmak", "alışverişe gitmek", "kitap okumak"];
import { Button, Checkbox, Form, FormControl, FormDescription, FormField, FormItem, FormLabel, Label, toast, useForm, z } from "@kampus/ui";
import { Link } from "lucide-react";
import { zodResolver } from "@hookform/resolvers/zod"

const meta = {
title: "Checkbox",
tags: ["autodocs"],
component: Checkbox,
argTypes: {
disabled: {
control: "boolean",
},
checked: {
control: "boolean",
}
},

} satisfies Meta<typeof Checkbox>;

export default meta;
type NewType = StoryObj<typeof meta>;
type Story = StoryObj<typeof meta>;

export const DefaultChecked = {
args: {
defaultChecked: true,
},
} satisfies Story;

export const OnFocus = {
Copy link
Contributor

Choose a reason for hiding this comment

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

@usirin if you can take a look at this and show a pointer to handle this correctly, that would be awesome!

-- we shouldn't manipulate DOM and focus is not doing what it meant to do, so this needs to be addressed but lets wait an answer from @usirin first

Copy link
Member

Choose a reason for hiding this comment

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

@melihcanclk Easiest would be creating a component inside this file and use state with proper onFocus handlers over there

Copy link
Contributor Author

@melihcanclk melihcanclk Aug 26, 2023

Choose a reason for hiding this comment

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

I tried to remove component inside OnFocus story and implement a component that makes handling with useState hook but realize that i have to give onFocus function outside of the component so to change the state inside onFocus function, i shouldn't make changes in Checkbox props, meaning it should take {...props} and with changing args to that component, we should do the change but this is my idea. If there is any way to change text inside the component by giving useState from args, I can implement it but this is what i did

Screenshot from 2023-08-26 11-04-15

I can commit this without the args at the down below in the story part or I can remove the whole onFocus part either because it really doesn't metter, why do i think in this way is that in Shadcn UI components' examples, there is no onFocus example. It has only With Text, Disabled and Form examples where all are implemented already, and if this is ok, i can do one extra thing that i did think that i could be doing but i didn't because of this same reason which is changing color of background and border of the checkbox over table down below.

Copy link
Contributor

Choose a reason for hiding this comment

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

I made some comments about this and I think the way you handled in the new version looks good to me! After you make those changes, we can review it again and merge this :) Thank you so much for the hard work and thinking about additions to the regular use cases 🚀 💯

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried to fix those and I'll be waiting for your feedback, thank you for your comments dear @cansirin 💜.

args: {
onFocus: () => {
const label = document.querySelector("#label");
label!.innerHTML = "Focused";
},
},
render: ({ ...props }) => (
<div className="flex flex-col space-y-2 ">
<div className="flex items-center space-x-2">
<Checkbox id="checkbox" {...props} />
<Label
htmlFor="checkbox"
className="text-sm font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70"
Copy link
Contributor

Choose a reason for hiding this comment

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

we need font color here too, please use text-primary

>
<span id="label">focus to trigger 'onFocus'</span>
</Label>
</div>
</div>
),
} satisfies Story;

export const WithText = {
argTypes: {
disabled: {
control: "boolean",
},
checked: {
control: "boolean",
}
},
render: ({ ...props }) => (
<div className="items-center flex space-x-2">
<Checkbox id="terms1" {...props} />
<div className="grid gap-1.5 leading-none">
<Label
htmlFor="terms1"
>
Accept terms and conditions
</Label>
<p className="text-sm text-muted-foreground">
You agree to our Terms of Service and Privacy Policy.
</p>
</div>
</div>
),
} satisfies Story;

export const InsideForm = {
render: ({ ...props }) => {
const FormSchema = z.object({
mobile: z.boolean().default(false).optional(),
})

function onSubmit(data: z.infer<typeof FormSchema>) {
toast({
title: "You submitted the following values:",
description: (
<pre className="mt-2 w-[340px] rounded-md bg-slate-950 p-4">
<code className="text-white">{JSON.stringify(data, null, 2)}</code>
</pre>
),
})
}

const form = useForm<z.infer<typeof FormSchema>>(
FormSchema,
{
resolver: zodResolver(FormSchema),
values: {
mobile: props?.checked,
}
},
)

return (
<Form {...form}>
<form onSubmit={form.handleSubmit(onSubmit)} className="space-y-6">
<FormField
control={form.control}
name="mobile"
render={({ field }) => (
<FormItem className="flex flex-row items-start space-x-3 space-y-0 rounded-md border p-4">
<FormControl>
<Checkbox
checked={field.value}
onCheckedChange={field.onChange}
{...props}
/>
</FormControl>
<div className="space-y-1 leading-none">
<FormLabel>
Use different settings for my mobile devices
</FormLabel>
<FormDescription>
You can manage your mobile notifications in the <Link href="/examples/forms">mobile settings</Link> page.
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the purpose of using Link component here? If it was for routing, let's not use Link, if it was for an icon, lets rearrange it so it won't be pushed to a new line.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In shadcn ui checkbox, there were some examples about checkbox and i tried to integrate those examples into our storybook but in form the link component was in use so i used it. It's my mistake that I didn't look how it should be look like in shadcn ui. I'll fix this now by removing Link component.

Copy link
Contributor

Choose a reason for hiding this comment

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

no problem at all! ty so much for the effort! 🚀

</FormDescription>
</div>
</FormItem>
)}
/>
<Button type="submit">Submit</Button>
</form>
</Form>
)
}
} satisfies Story;


export const Default = {
render: () => (
args: {
children: "Default"
},
render: (
{ children, ...props }
) => (
<div className="flex flex-col space-y-2 ">
<h4>Günlük Yapmam Gerekenler</h4>
{labels.map((label) => {
return (
<div className="flex items-center space-x-2">
<Checkbox id={label} />
<Label
htmlFor={label}
className="text-sm font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70"
>
{label}
</Label>
</div>
);
})}
<div className="flex items-center space-x-2">
<Checkbox id="checkbox" {...props} />
<Label
htmlFor="checkbox"
className="text-sm font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70"
>
{children}
</Label>
</div>
</div>
),
};
} satisfies Story;