Skip to content

feat(Label): added toggletip props in label #1107

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

Merged
merged 2 commits into from
Mar 13, 2025
Merged

feat(Label): added toggletip props in label #1107

merged 2 commits into from
Mar 13, 2025

Conversation

JsGarneau
Copy link
Contributor

Normalement dans une autre PR on pourrait retirer les tooltips

@JsGarneau JsGarneau requested a review from a team as a code owner March 11, 2025 15:44
@JsGarneau JsGarneau changed the title feat(Bunch of Inputs): added toggletip props in label feat(Label): added toggletip props in label Mar 11, 2025
Copy link

Storybook for this build: https://ds.equisoft.io/pr-1107/

Copy link

Webapp for this build: https://ds.equisoft.io/pr-1107/webapp/

Copy link
Contributor

@LarryMatte LarryMatte left a comment

Choose a reason for hiding this comment

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

Quelques petits commentaire pour bien aligner le label et le toggleTip

Dans le StyledLabel du fichier label.tsx, remplace le line-height pour toujours avoir 1.5rem.

const StyledLabel = styled.label<{isMobile: boolean}>`
    ...
    line-height: 1.5rem;
    ...
`;

Dans le StyledDiv du fichier FieldContainer.tsx remplace le --spacing-half du margin-bottom par --spacing-quarter.

> :nth-child(${({ hasLabel, hasHint, valid }) => (hasLabel ? 1 : 0) + (hasHint ? 1 : 0) + (!valid ? 1 : 0)}) {
        margin-bottom: var(--spacing-quarter);
    }`

import { Tooltip, TooltipProps } from '../tooltip/tooltip';
import { useTranslation } from '../../i18n/use-translation';

const StyledDiv = styled.div`
align-items: center;
align-items: end;
Copy link
Contributor

Choose a reason for hiding this comment

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

Le toggleTip n'est pas vraiment centré avec le label.
Screenshot 2025-03-12 at 4 16 18 PM

Garde le center du align-items:.
Je vais te donner quelques changements pour que la vie soit belle, que le toggleTip soit là ou non.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ajoutes un gap: var(--spacing-half);
Ça va permettre de ne pas avoir les margin pour le tooltip et toggleTip.
Tu vas pouvoir supprimer const StyledTooltip et const StyledToggletip et utiliser directement le tooltip et toggletip sans les styler.

@@ -28,13 +30,18 @@ const StyledTooltip = styled(Tooltip)`
margin-left: calc(var(--spacing-1x) * 1.5);
`;

const StyledToggletip = styled(Toggletip)`
margin-left: calc(var(--spacing-half) * 1.5);
Copy link
Contributor

@LarryMatte LarryMatte Mar 12, 2025

Choose a reason for hiding this comment

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

Tu peux supprimer ici et même celui du tooltip.
Le gap, que je te fais ajouter dans le StyledDiv plus haut vient ajouter l'espace nécessaire entre les éléments.
(voir commentaire dans le StyledDiv: #1107 (comment))

Copy link
Contributor

@meriouma meriouma left a comment

Choose a reason for hiding this comment

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

LGTM, resterait juste à corriger les commentaires de Larry!

@JsGarneau JsGarneau merged commit 68cb4f9 into master Mar 13, 2025
21 checks passed
@JsGarneau JsGarneau deleted the fix/DS-1337 branch March 13, 2025 18:52
meriouma pushed a commit that referenced this pull request Mar 17, 2025
* feat(Label): added toggletip props in label

* fix(Inputs): css tweaks on toggletip
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.

None yet

3 participants