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(Infos): allow ReactNode in word and definition infos #965

Merged
merged 3 commits into from
Jul 22, 2022

Conversation

MartinWeb
Copy link
Contributor

@MartinWeb MartinWeb commented Apr 29, 2022

Related issue

The Infos component doesn't allow to put a style on the word or the definition. It could be nice to give that opportunity to allow the consumer to style one particular definition or word.

Reference to the issue

#964

Person(s) for reviewing proposed changes

@arnaudforaison @samuel-gomez

);
expect(asFragment).toMatchSnapshot();
Copy link
Contributor

Choose a reason for hiding this comment

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

on supprime le snapshot ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je suis parti pour le supprimer et faire plutôt un test via testing-library, mais si on veut le conserver ça ne me dérange pas.

Copy link
Contributor

Choose a reason for hiding this comment

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

non pas de souci, par contre tu peux vérifier tous les autres éléments (definition, id), stp. ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je fais ça

Copy link
Contributor Author

@MartinWeb MartinWeb Apr 29, 2022

Choose a reason for hiding this comment

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

Fait, seul l'id pour lequel je ne vois pas comment le tester, car il est utilisé uniquement dans les attributs key qui n'apparaisse pas dans le dom et donc ne sont pas atteignables. Sauf si tu vois une manière de le faire que je ne connaitrais pas. En tout cas, le coverage est à 100%.

@arnaudforaison arnaudforaison merged commit e9deda8 into AxaFrance:master Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants