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

adjust the list styles to accommodate MWS #2335

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

Conversation

polmih
Copy link
Contributor

@polmih polmih commented Mar 28, 2024

The ul css most common classes can be applied on parent div too in order to accommodate MWS limitation

duboisp

This comment was marked as resolved.

@polmih
Copy link
Contributor Author

polmih commented Apr 4, 2024

Corriger les erreurs du CI et adresser l'ensemble des commentaires inline.

C'est fait. Merci! :)

Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

See the inline comments.

common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-fr.html Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-fr.html Outdated Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
common/list/_lists.scss Show resolved Hide resolved
common/list/_lists.scss Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
@Garneauma
Copy link
Contributor

Pre-approved upon successful review.

@duboisp duboisp added this to the v14.9.0 milestone Apr 22, 2024
@duboisp
Copy link
Member

duboisp commented Apr 22, 2024

See the inline requested change

And the following must be fixed. There is extra spaces added at the top of pages

Issue:
image

Expected
image

Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

And see the top margin issue on our template

common/list/_lists.scss Outdated Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
@polmih
Copy link
Contributor Author

polmih commented May 1, 2024

And see the top margin issue on our template

Fixed. Thanks

@polmih polmih requested a review from duboisp May 1, 2024 12:50
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

See the requested change,

Note for myself regarding the next review: I will need to go through all the preceding comment to ensure they have been addressed.

common/list/lists-mws-en.html Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
common/list/lists-mws-fr.html Outdated Show resolved Hide resolved
sites/baseline/lists/_lists.scss Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Please add/adjust the working example of ul.disc / ul.circle / ul.square

Thanks

common/list/_lists.scss Outdated Show resolved Hide resolved
@polmih
Copy link
Contributor Author

polmih commented May 13, 2024

Please add/adjust the working example of ul.disc / ul.circle / ul.square

Thanks

Fait. Merci

Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

There is a few editorial change, see my inline comment + you will need to apply the similar change to the French version.

And, let's remove the extra CSS class for when "disc", "circle", "square" is directly used on the unordered list element compared to when it is used on a generic container

common/list/_lists.scss Outdated Show resolved Hide resolved
common/list/_lists.scss Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
common/list/lists-mws-en.html Outdated Show resolved Hide resolved
@polmih
Copy link
Contributor Author

polmih commented May 14, 2024

There is a few editorial change, see my inline comment + you will need to apply the similar change to the French version.

And, let's remove the extra CSS class for when "disc", "circle", "square" is directly used on the unordered list element compared to when it is used on a generic container

Changes applied. Thanks

@polmih polmih requested a review from duboisp May 14, 2024 16:36
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.

None yet

3 participants