feat: update SampleTable to use Form fields to match parent field

[frankie-mur]

Description

• This PR updates the SampleTable.tsx file to use the Form* componenets, this addresses the issue #2470 and matches the text style, size, and formatting with the rest of the parent Form.

• I was not able to test the sample button press and calling openAPI with loading data into the table as I do not have an paid openAPI key and it seems free plan does not support :( , should not effect this but if someone can confirm that would be appreciated

• I am very novice with React so please feel free to give tips and pointers where needed

• Thanks 👍

[vercel[bot]]

@frankie-mur is attempting to deploy a commit to the neosync Team on Vercel.

A member of the Team first needs to authorize it.

[nickzelei]

Hi @frankie-mur ! Appreciate the PR.

Do you think it would be possible to update this component to use similar styles without using the Form components directly? This table isn't a part of the form itself so I would prefer if it did not use the form components.

Otherwise, if it's a little funky to do so, am happy to merge it as-is.

[frankie-mur]

Thanks for the review @nickzelei , I have updated much simpler and without using Form components, originally I had assumed it was apart of the form, should of double checked that.

[evisdrenova]

@frankie-mur here is the styling for the subtext for the other form fields, we'll just want them to look the same.

const FormDescription = React.forwardRef<
  HTMLParagraphElement,
  React.HTMLAttributes<HTMLParagraphElement>
>(({ className, ...props }, ref) => {
  const { formDescriptionId } = useFormField();

  return (
    <p
      ref={ref}
      id={formDescriptionId}
      className={cn('text-[0.8rem] text-muted-foreground', className)}
      {...props}
    />
  );
});
FormDescription.displayName = 'FormDescription';

[frankie-mur]

Appreciate that @evisdrenova, should we also update the heading Synthetic Data Sample to match the other form fields? Also since we don't want use Form components directly would it be appropriate to use the same className attributes directly? ex.

     <p className="text-[0.8rem] text-muted-foreground tracking-tight">
            A sample of synthetic data given the inputs above. Returns at most
            10 records.
     </p>

[nickzelei]

Appreciate that @evisdrenova, should we also update the heading Synthetic Data Sample to match the other form fields? Also since we don't want use Form components directly would it be appropriate to use the same className attributes directly? ex.

     <p className="text-[0.8rem] text-muted-foreground tracking-tight">
            A sample of synthetic data given the inputs above. Returns at most
            10 records.
     </p>

Yep it is totally appropriate to use the same class names! These are just tailwind classes so it's totally fine to use those as they are used for rendering purposes.

We could update the heading, however, it's not intended to stand out as it's a separate part of the form. However, if it looks better uniform with the others, I'm down for it!

[frankie-mur]

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
neosync-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 19, 2024 7:39pm