patternfly / patternfly/patternfly-react

Review <Icon> usage

Abierto
#11,195 5 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

PF Team Stale
Lenguaje dominante
TypeScript
Estrellas
862
Forks
392
Merge medio
4 d 8 h
PR fusionados (30 d)
9

Descripción

A follow up from https://github.com/patternfly/patternfly-react/pull/11005

In the backstop run for the above PR, I spotted an issue we should look into.

Screenshot 2024-11-14 at 1 24 52 PM

That's from this right-to-left table demo and this file - https://github.com/patternfly/patternfly-react/blob/6b56f7f777129a15c515719a83d8fb45fa812322/packages/react-core/src/demos/RTL/examples/PaginatedTable.tsx#L259-L270

We use <Icon shouldMirrorRTL /> as the preferred way of including an icon that should mirror/flip if the document is set to dir="rtl", so I'm thinking we need to keep that there, and add isInline to let the <Icon> inherit the color and font-size from its parent instead of using our default icon color/size.

And just looking at that demo, looks like we have that same issue in the labels, too. For example, this one https://github.com/patternfly/patternfly-react/blob/6b56f7f777129a15c515719a83d8fb45fa812322/packages/react-core/src/demos/RTL/examples/PaginatedTable.tsx#L212-L221

I updated the first label in the table and added .pf-m-inline to the .pf-v6-c-icon in dev tools and you can see it resized the icon since icons in labels are smaller than our default icon size

Screenshot 2024-11-14 at 1 39 48 PM

I'm not sure how big of a lift it would be, but I would probably suggest we just review each use of <Icon> in the examples/demos and if it's used in a component that wants to style the icon color/size, we consider 1) removing <Icon> if it isn't necessary, and 2) adding isInline if it is necessary.

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Comienza con packages/react-core/src/demos/RTL/examples/PaginatedTable.tsx, especialmente con los paginated-table labels y controls referenciados, y revisa el backstop result relacionado. Comprueba cada uso de Icon en los examples y demos para detectar wrappers innecesarios o la ausencia de isInline styling. Se considera terminado cuando los icons afectados heredan el color y el tamaño de su parent, manteniendo el RTL mirroring donde sea necesario.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
react, typescript
Área
frontend
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
35/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.