Review <Icon> usage

Ouverte
#11,195 5 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Évaluation

Difficulté
4/5
Temps estimé
3-5 jours
Accessibilité débutants
35/100
Type d'issue
Bug
Clarté
Plutôt claire
Activité
À l'abandon
Stack technique
react, typescript
Domaine
frontend

Piste de recherche

Commencez par packages/react-core/src/demos/RTL/examples/PaginatedTable.tsx, en particulier les paginated-table labels et controls référencés, et examinez le backstop result associé. Vérifiez chaque utilisation de Icon dans les examples et demos pour détecter d’éventuels wrappers inutiles ou l’absence de isInline styling. C’est terminé lorsque les icons concernés héritent de la couleur et de la taille de leur parent tout en conservant le RTL mirroring lorsque nécessaire.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Description

PF Team Stale

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.

Langage dominant
TypeScript
Étoiles
862
Forks
392
Merge moyen
4 j 8 h
PR mergées (30 j)
9

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Autres issues de patternfly/patternfly-react

Toutes les issues de patternfly/patternfly-react

Issues similaires

Plus d'issues TypeScript

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.