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

Pretty links #675

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Pretty links #675

merged 6 commits into from
Oct 10, 2023

Conversation

alinekeller
Copy link
Contributor

Suppression des styles spéciaux pour les liens avec la classe '.link-pretty'. La propriété 'text-decoration-color', qui permet d'obtenir le même effet sans avoir recours à des styles complexes et difficiles à maintenir, est désormais suffisamment bien implémentée dans les navigateurs web.

https://epfl-webvolution.atlassian.net/browse/WEBEVOL-211

@github-actions
Copy link

github-actions bot commented Jul 27, 2023

Unit Test Results

    1 files      1 suites   0s ⏱️
265 tests 230 ✔️ 0 💤   0  35 🔥
265 runs  195 ✔️ 0 💤 35  35 🔥

For more details on these errors, see this check.

Results for commit a441160.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jul 27, 2023

🔎 Download the Backstop report for this pull request (link valid for 90 days):

Comment on lines +69 to +70
.bg-dark .link-pretty,
.bg-gray-600 .link-pretty{
Copy link
Member

Choose a reason for hiding this comment

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

Après plusieurs tests, il me semble qu'il manque des classes pour les variantes de background.
Par exemple, une alerte avec bg-danger text-white et un lien.

@each $color, $value in $theme-colors {
.bg-#{$color} & {
color: color-yiq($value);
@include link-pretty-text-shadow($value);
@if ($hover) {
&:hover,
&.hover {
background-image: linear-gradient(gray('300') 0%, gray('300') 100%);
}
}
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pour les couleurs d'arrière-plan générées automatiquement, je propose de simplement reprendre la couleur du texte pour le soulignement (en rouge ça rend mal sur tous les fonds à part le noir). La fonction Bootstrap color-yiq permet d'adapter automatiquement la couleur du texte à celle du fond, pour avoir le meilleur contraste:

epfl-links-02

Idem pour les liens avec la classe .text-white:

epfl-links-01

@williambelle @xentenza est-ce que ça vous semble ok comme ça ?

Copy link
Member

Choose a reason for hiding this comment

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

Selon tes captures, ça me semble très bien.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xentenza Est-ce que ça te convient ? Je viens de me rendre compte que cette PR est toujours ouverte, j'aurais besoin de ta validation pour finaliser les changements :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@alinekeller oups... c'est tout bon!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xentenza @williambelle Je viens d'envoyer les corrections que j'avais proposées. Vous pouvez voir ce que ça donne (notamment les états :hover et :focus) sur la page atoms/link. La section principale ne contient plus que les trois exemples standard, sur fond gris clair et sur fond gris foncé. J'ai ajouté une section supplémentaires pour les variantes de background.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@alinekeller Merci pour ta PR. Vus à la suite des liens standards, le soulignement qui disparaît dans les background dark et colorés fait bizarre, mais je ne vois pas comment tu pourrais faire autrement.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@alinekeller Le bouton "Primary small" dans Atoms > Button est plus petit, ce qui introduit plusieurs modifications en cascade:

  • Atoms > Upload
  • Atoms > Collapse > Partial Collapse
  • Content Types > Basic Page
  • etc.

Si ce n'est pas voulu et tu arrives à mettre la taille d'origine, super. Sinon, on fait avec, ce n'est pas hyper dérangeant

Copy link
Member

Choose a reason for hiding this comment

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

Il me semble que c'est à cause de la branche qui n'était pas à jour.

@williambelle williambelle merged commit 4e7eca4 into dev Oct 10, 2023
4 checks passed
@williambelle williambelle deleted the styleguide/pretty-links branch October 10, 2023 14:20
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