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

RonRonnement CSS: Fix small discontinuity in the left sidebar #354

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

Conversation

nud
Copy link
Contributor

@nud nud commented Jan 5, 2023

This PR fixes this bug in Firefox:

image

See commit messages for details.

Commit 88aa3c3 included a small
computation mistake: the border should be taken into account in the
calculations.

Also, while it looks correct in Chrome, for some reason Firefox does not
properly position the linear gradient origin when the background is set
to `repeat`. Funnily it doesn't exhibit the same behaviour when using
the built-in screenshot tool. We add `no-repeat` to the background
specification to fix this.

We also remove the backgrond of the sidebar as it is useless since it is
the same colour as the body background, but it makes the miscalculation
obvious should one happen again.
@nud
Copy link
Contributor Author

nud commented Jan 17, 2023

@Trim pas intéressé?

@Trim
Copy link
Member

Trim commented Jan 17, 2023

Si c'est intéressant, je n'ai pas eu le temps de regarder.

Je suis surpris de la solution qui fait "+1px", alors je voulais vérifier s'il n'y pas une solution qui corrigerait de manière plus logique.

Je ne suis pas très habitué aux règles du "responsive design" et j'ai donc un doute si ça passe sur les différentes tailles d'écran.

@nud
Copy link
Contributor Author

nud commented Jan 17, 2023

Le 1px est la taille de la bordure. La coordonnée (0,0) est le coin supérieur gauche du div et ce coin est occupé par la bordure, mais l'arrière-plan y est déjà. Comme la barre est à l'intérieur de la bordure il faut ajouter le 1px de la bordure pour tomber juste. J'aurais aussi pu mettre la largeur de la bordure dans une variable pour rendre les choses plus explicites mais bon...

Sinon si c'est le 1px qui te choque tu peux l'enlever: comme le commit enlève aussi le background de la sidebar, une erreur de calcul de 1px passera inaperçue.

Pour ce qui est du responsive, je ne pense pas que ça change quoi que ce soit dans la mesure où la sidebar a une largeur exprimée en px et que cette largeur est reprise dans la définition du gradient.

Trim added a commit to Trim/linuxfr.org that referenced this pull request Jan 17, 2023
The issue is the linear-gradient in the body background seems to not working
really correctly with pixel dimensions (in Firefox at least): if you
zoom in / out the page you'll see the body background to not always
correctly update the rendering (and so you'll see a growing gasp for
example).

Due to this issue, there was a gap between the backgrounds of the body and the
sidebar elements. PR linuxfrorg#354 tried to solve this by adjusting the
linear-gradient pixel dimensions, but due to the bug of linear-gradient
implementation, it was not working when window was resized or page zoomed.

As the linear-gradient usage in the body background was clearly already a
workaround to create two vertical color stripes, we replace it with
another workaround using the CSS feature which allows to define multiple
background images with different size and position.

First background image is defined with the sidebar color (using
linear-gradient with only one color) and the width which resolve the
pixel gasp reported in linuxfrorg#354 (which means the width should be the
branding width + body border width). To be able to define a color as an
image, we use again linear-gradient, but, this time it fills all the
stripe with the same color, so we won't have error due to
linear-gradient implementation.

Second image is defined from the first image position for the rest of the body
block with the container color.

This new workaround idea comes from this StackOverflow answer which
explain how to create pixel perfect horizontal stripes:
https://stackoverflow.com/a/24829344
@Trim
Copy link
Member

Trim commented Jan 17, 2023

Le 1px est la taille de la bordure. La coordonnée (0,0) est le coin supérieur gauche du div et ce coin est occupé par la bordure, mais l'arrière-plan y est déjà. Comme la barre est à l'intérieur de la bordure il faut ajouter le 1px de la bordure pour tomber juste. J'aurais aussi pu mettre la largeur de la bordure dans une variable pour rendre les choses plus explicites mais bon...

Merci pour l'explication du 1px, c'est bien ça et c'est défini dans le même bloque :)

Sinon si c'est le 1px qui te choque tu peux l'enlever: comme le commit enlève aussi le background de la sidebar, une erreur de calcul de 1px passera inaperçue.

J'ai essayé sans le background et sans le 1px, mais, quand tu es déconnecté, tu vois à nouveau l'erreur parce que le formulaire de login affiche un "border-bottom" qui dépasse de 1px de la sidebar. C'est le serpent qui se mord la queue 😓

Pour ce qui est du responsive, je ne pense pas que ça change quoi que ce soit dans la mesure où la sidebar a une largeur exprimée en px et que cette largeur est reprise dans la définition du gradient.

C'est ce que je pensais aussi, mais l'implémentation du zoom out de Firefox et GNOME Web montre que le rendu de la sidebar ne suit pas bien le zoom (à cause de linear-gradient apparemment).

J'ai trouvé un nouveau workaround que j'ai implémenté dans #357 qui fonctionne bien pour Chrome, Webkit (enfin GNOME Web) et Firefox.

@nud
Copy link
Contributor Author

nud commented Mar 6, 2024

@Trim Do we merge this or close this and merge #357 instead? My OCD is still triggered by a bug that's been fixed for a year.

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