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

#478 change list decoration placement logic #542

Merged

Conversation

syjer
Copy link
Contributor

@syjer syjer commented Aug 28, 2020

This PR implement what has been done in the issue #478 .

I'll add another test (especially on the RTL side of the equation) before marking the PR as done :).

@syjer syjer force-pushed the 478-li-style-decoration-positioning branch from 93b002c to 6642768 Compare August 29, 2020 13:45
@syjer syjer force-pushed the 478-li-style-decoration-positioning branch from 4c07c6d to 5600297 Compare August 29, 2020 16:02
@syjer syjer marked this pull request as ready for review August 29, 2020 16:02
@syjer
Copy link
Contributor Author

syjer commented Aug 29, 2020

Hi @danfickle , I've marked this PR as ready. I've added a new test which check with various fonts and sizes the RTL case. Like all the browsers, there will be cases where the marker will not be able to be put inside the default padding, so it will be cut (partially or totally).

To be noted, as written in #478, we need to add the support for padding-inline-start updating the default css for a more out of the box experience for the RTL case, as I added an explicit right padding in the test.

@danfickle
Copy link
Owner

Looks good. Thank you. I'm working on an exciting feature (pr soon) but after that I'll take a look at padding-inline-start and friends.

@danfickle danfickle merged commit 3f42c9a into danfickle:open-dev-v1 Aug 31, 2020
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

2 participants