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

FontAwesome: Typescript issue passing props #6564

Closed
jnoren1 opened this issue May 3, 2024 · 6 comments · Fixed by #6579, leoo1992/GeradorQRCode#40, mtech-31-quemistry/quemistry_client_web#3 or leoo1992/GeradorQRCode#57 · May be fixed by nhattpn/BTL_LTNC#56
Assignees
Labels
Typescript Issue or pull request is *only* related to TypeScript definition
Milestone

Comments

@jnoren1
Copy link

jnoren1 commented May 3, 2024

Describe the bug

It appears that Chip is not passing the key property down to its children? The following message is caused by the following code.

"Warning: Each child in a list should have a unique "key" prop.
Check the render method of Chip. "

 <Chip
          key={program.id}
          label={program.subgroup ? `${program.subgroup} - ${program.name}` : program.name}
          className="p-chip-rounded"
          removable
          removeIcon={
            <FontAwesomeIcon
              icon={faXmarkCircle}
              className="ml-2 text-red-400 hover:text-red-950 cursor-pointer"
              size="1x"
              onClick={(e) => {
                onProgramRemove(program);
                e.preventDefault();
              }}
            />
          }
          onRemove={() => onProgramRemove(program)}
        />

the following resolves the issue...

<Chip
         key={program.id}
         label={program.subgroup ? `${program.subgroup} - ${program.name}` : program.name}
         className="p-chip-rounded"
         removable
         removeIcon={
           <FontAwesomeIcon
             key={`${program.id}-${index}`}
             icon={faXmarkCircle}
             className="ml-2 text-red-400 hover:text-red-950 cursor-pointer"
             size="1x"
             onClick={(e) => {
               onProgramRemove(program);
               e.preventDefault();
             }}
           />
         }
         onRemove={() => onProgramRemove(program)}
       />

I use custom icons all over the place and never need to do this, so I think this is a failing of Chip. Not sure though.

Reproducer

https://stackblitz.com/edit/vitejs-vite-kn9li3?file=src%2FApp.tsx

PrimeReact version

10.6.3

React version

17.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

Chrome Latest

Steps to reproduce the behavior

run code, open console. see error. adding a key to the icon fixes it.

Expected behavior

no warning.

Also, the onRemove function does not seem to work with the custom icon. I expect that to work, but maybe I misunderstand it's purpose.

@jnoren1 jnoren1 added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label May 3, 2024
@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels May 3, 2024
@melloware

This comment was marked as outdated.

@melloware
Copy link
Member

@jnoren1 can you try my reproducer: https://stackblitz.com/edit/vitejs-vite-tfhpc2?file=src%2FApp.tsx

You missed one step {...options.iconProps} I think https://primereact.org/customicons/#fontawesome

@melloware melloware added Resolution: Needs More Information More information about the issue is needed to find a correct solution and removed Type: Bug Issue contains a defect related to a specific component. labels May 3, 2024
@jnoren1
Copy link
Author

jnoren1 commented May 4, 2024

Wow, I've been using this library for about six years, and I never came across that specific piece of documentation. When we noticed the JSX indicators, we wondered if we could incorporate a custom Icon. Through trial and error it turned out we could, and we just went ahead with it without consulting the documentation. Your approach did indeed solve that issue. Miraculously, it also corrected the button callbacks, which had never quite worked as described. That's fantastic, thank you so much.

It opened up another more concerning error though. There is some typescript incompatibility going on with the size property and when forced it gave the following. I've added the callback style now to a lot of code and every single one takes issue with the size....

ProgramChips.tsx?t=1714800159507:20 Warning: Failed prop type: Invalid prop size of value 4 supplied to FontAwesomeIcon, expected one of ["2xs","xs","sm","lg","xl","2xl","1x","2x","3x","4x","5x","6x","7x","8x","9x","10x"].

I hacked it a wrapper function, but that does not seem ideal

    removeIcon={(i) => (
            <FontAwesomeIcon
              {...faExProps(i.iconProps)}
              key={`${program.id}-${index}`}
              icon={faXmarkCircle}
              className="ml-2 text-red-400 hover:text-red-950 cursor-pointer"
              size={"xl"}
            />
          )}
export const faExProps = (props: HTMLProps<unknown> | SVGProps<unknown>): { [key: string]: unknown; } => {
  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  const { size, ...rest } = props as HTMLProps<unknown>;
  return rest;
};

@melloware
Copy link
Member

melloware commented May 4, 2024

@jnoren1 that is great. Do you have a suggestion how we could update the Typescript to make it flexible right now as you know its...

Right now the Typescript is.

/**
 * Icon options passed to any icon.
 * ComponentProps are props from the owning component.
 * AdditionalProps are any custom properties of an icon like SortIcon of the Datatable for example.
 */
export type IconOptions<ComponentProps, AdditionalProps> = AdditionalProps & {
    /**
     * Icon specific properties.
     */
    iconProps: React.HTMLProps<any> | React.SVGProps<any>;
    /**
     * The element representing the icon.
     */
    element: React.ReactNode;
    /**
     * Properties of the owning component.
     */
    props?: ComponentProps;
    [key: string]: any;
};

@melloware melloware added Typescript Issue or pull request is *only* related to TypeScript definition and removed Resolution: Needs More Information More information about the issue is needed to find a correct solution labels May 4, 2024
@melloware melloware changed the title Chip not passing key with custom remove icon FontAwesome: Typescript issue passing props May 4, 2024
melloware added a commit to melloware/primereact that referenced this issue May 6, 2024
@melloware melloware self-assigned this May 6, 2024
@melloware melloware added this to the 10.7.0 milestone May 6, 2024
melloware added a commit to melloware/primereact that referenced this issue May 6, 2024
@melloware
Copy link
Member

@jnoren1 can you check my PR I think I fixed it to FontAwesome size property will be respected. #6579

@melloware
Copy link
Member

@jnoren1 10.6.6 is out if you want to check the new typescript typing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment