WIP: Resolve "Picture and text misaligned in namespace selection buttons"
What does this MR do?
This changeset adds an override to some breaking CSS that causes .avatar-container
s to not be centered when presented on the fork namespace selection screen.
I'm not sure exactly when this was broken - if it was ever right - but here's the traceback of the intent:
- The
.fork-thumbnail .avatar-container
is supposed to be centered .avatar-container
extends.avatar-circle
- All
.avatar-circle.s*
s have an explicit pixel value right margin (provided by theavatar-size
mixin)
Notably, this change is "rolling back" some CSS applied in that above chain by overriding it with conflicting properties in a rule of slightly higher specificity. Note that .mx-auto
is applied to the .avatar-container
- presumably in an attempt to maintain the centering - but because its specificity is so low (010) the offending rule is still overriding it. The rule that applies the right margin is .s*.avatar-container
(020) and the newly added rule that sets it back to the intended auto
value is .fork-thumbnail .avatar-container.s*
(030).
As an aside: I am fairly strongly of the opinion that avatar-size
should not be applying a margin. The principle of least astonishment dictates that if something is named size
it shouldn't also affect the margin
.
An aside to the aside: The correct fix here is to componentize the avatar so that the styles cannot cross the component boundary (Shadow DOM!?!) and then provide a way to opt in/out of the margin at the component level. That fix, however, would be a very significant overhaul of every place that uses the avatar, and would be very, very far outside the scope of this issue.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry for user-facing changes, or community contribution. Check the link for other scenarios. -
Documentation created/updated or follow-up review issue created -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Performance and Testing
I chose to override the offending rule with a rule of higher specificity because there are dozens of places that have the .avatar-container.s*
class, while there is only one place where .fork-thumbnail .avatar-container.s*
applies. By making only this single change, I drastically reduce the risk of unintended side effects elsewhere. However, this is slightly less efficient, since the browser will apply the previous style and then re-apply the fix. If a user has many thousands of potential namespaces (is this possible??) it may have a barely noticeable impact.
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
~~Label as security and @ mention @gitlab-com/gl-security/appsec
~~ -
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Closes #64068 (closed)