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

fix(ordered-list): fix list alignment - FRONT-3744 #2650

Merged
merged 5 commits into from
Oct 6, 2022
Merged

Conversation

emeryro
Copy link
Contributor

@emeryro emeryro commented Sep 22, 2022

Apply new guidelines for ordered list alignment: small indent for the first 9 items, so it is aligned with the paragraph if there are 10+ items.
This can be tested on https://single-market-economy.ec.europa.eu/single-market/single-market-services/economic-analysis_en
image

Note: it is not possible to use "inside" list style, as it would align the numbers to the left, and would prevent us from having custom padding between the number and the text

@github-actions
Copy link

github-actions bot commented Sep 22, 2022

@@ -24,9 +24,12 @@ $_text-color: null !default;

.ecl-ordered-list__item,
%ecl-ordered-list__item {
margin-inline-start: map.get(theme.$spacing, 'm');
// align items up to two digits
margin-inline-start: 1.75em;
Copy link
Contributor

Choose a reason for hiding this comment

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

wow, but are we sure we want do this..? It's true that the aligment of a long ordered list is improved (at least in the major browsers) but the same applied to a short list is actually going to make it worst than it currently is, no..?

You said we cannot use list-inside because of the "custom" padding but it seems to me that in order to "preserve" this minor style we compromise the behaviour of a standard html tag.

I feel that the way to go would actually be to remove this customisation and just stick to the default external or internal styles of the list (potentially ony for the ordered, the unordered should not present the same problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we use inside numbers, they are aligned on the left
image
We can discuss it with COMM next meeting, but that's not what they mentioned in the requirements

Copy link
Contributor

@planctus planctus Sep 22, 2022

Choose a reason for hiding this comment

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

but we are never going to fulfill these requirements, from what i understand, it looks to me that here is a matter of avoiding the "bad alignments" that are due to our customisation, basically.
A list doesn't look too good nor too bad in html, it is what it is, outside or inside aligned, but with our "little" overrides we are breaking this alignment, both in the current implementation and in this proposal, which makes it more evident increasing the margin that we force for an external list to almost look like if it's an internal one.
Then sure, it's their decision, but i would push for something were the alignment is not meant to work on a single example with 14 items only. :)

@emeryro emeryro merged commit 68be8f1 into v3-dev Oct 6, 2022
@emeryro emeryro deleted the FRONT-3744-list branch October 6, 2022 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants