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

Create variant for link with 'btn' class #704

Merged
merged 4 commits into from
Feb 13, 2024
Merged

Create variant for link with 'btn' class #704

merged 4 commits into from
Feb 13, 2024

Conversation

alinekeller
Copy link
Contributor

Correction de la couleurs des liens avec la classe 'btn' (https://epfl-webvolution.atlassian.net/browse/WEBEVOL-240)

Copy link

github-actions bot commented Jan 10, 2024

Unit Test Results

    1 files      1 suites   0s ⏱️
268 tests 264 ✔️ 0 💤 0  4 🔥
268 runs  260 ✔️ 0 💤 4  4 🔥

For more details on these errors, see this check.

Results for commit af932dd.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 10, 2024

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

Copy link
Member

@williambelle williambelle left a comment

Choose a reason for hiding this comment

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

De mon côté, c'est difficile de juger si la couleur noire est juste ou fausse selon les situations. Par exemple, le hover sur un élément visité ou alors la pagination : Screenshot from 2024-01-11 12-49-10

Pour les liens, je crois qu'on peut aussi utiliser : btn-success, btn-danger, btn-warning, btn-info, btn-light, btn-dark et btn-link.

@obieler
Copy link
Member

obieler commented Jan 29, 2024

Salut @alinekeller,
Est-ce que tu as vu la révision de @williambelle?

@alinekeller
Copy link
Contributor Author

@williambelle @obieler Après avoir fait une nouvelle batterie de tests, je pense qu'il vaut finalement mieux retirer les changements de la PR #694. Le problème reporté avec les liens :visited concerne uniquement un composant WordPress, pour lequel j'ai par ailleurs proposé des changements de design (epfl-si/wp-theme-2018#308).

Dans le styleguide, les liens :visited se comportent par défaut comme des liens standards, ça convient très bien. Et Bootstrap rend ce genre de changements un peu complexe, il aurait fallut redéfinir la couleur de base des liens dans les styles de l'atom, ce qui est redondant.

À mon avis il vaut donc mieux revenir à la situation initiale. J'ai retiré les styles que j'avais ajoutés pour le liens qui ont la classe .btn, .btn-primary et .btn-secondary. Je conserverait en revanche la variante Button Link dans le styleguide: la confusion entre les balises <a> et <button> est un problème très courrant pour l'accessibilité, il me paraît donc pertinent d'avoir cette mention des cas où un vrai bouton devrait être utilisé plutôt qu'un lien avec une apparence de bouton.

Désolée pour ces allers-retours.

Comment on lines 46 to 49
.page-link,
.page-link:visited {
color: $red;
}
Copy link
Member

@williambelle williambelle Feb 8, 2024

Choose a reason for hiding this comment

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

Je pense qu'on peut supprimer suite au "revert" du a:visited.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Effectivement, c'est corrigé.

@williambelle williambelle merged commit 8a5f63f into dev Feb 13, 2024
4 checks passed
@williambelle williambelle deleted the fix/btn-link branch February 13, 2024 11:40
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