Skip to content

Fix usage of div in CallOut #399

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

benjlevesque
Copy link
Contributor

Similar to #394, but for CallOut

NB: this issue seems to exist for many components

@ddecrulle
Copy link
Collaborator

ddecrulle commented Mar 28, 2025

Hi,
I don’t believe this properly fixes the issue.

In my opinion, we should improve the component’s design: it should either accept a text prop and render a <p> with the appropriate className, or directly render its children without wrapping them in an extra <div>.

@benjlevesque
Copy link
Contributor Author

benjlevesque commented Mar 28, 2025

Hello @ddecrulle

do you have something like this in mind ?

{typeof children === "string" ? (
    <p className={cx(fr.cx("fr-callout__text"), classes.text)}> {children} </p>
) : (
    <div className={cx(fr.cx("fr-callout__text"), classes.text)}> {children} </div>
)}

children having the type ReactNode

@ddecrulle
Copy link
Collaborator

ddecrulle commented Mar 28, 2025

This implementation is close, but not exactly what we need.
The main issue is that it doesn’t allow us to render the HTML structure provided in the DSFR documentation, like this example:

<div id="callout-6045" class="fr-callout">
    <h3 class="fr-callout__title">Titre mise en avant</h3>
    <p class="fr-callout__text">Lorem [...] elit ut.</p>
    <button type="button" class="fr-btn">Libellé bouton</button>
</div>

It's tricky to improve the current component without introducing a breaking change.

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.

2 participants