Skip to content
This repository has been archived by the owner on Mar 6, 2022. It is now read-only.

Add support for list items #7

Merged
merged 3 commits into from
Jul 22, 2020
Merged

Add support for list items #7

merged 3 commits into from
Jul 22, 2020

Conversation

dotanrs
Copy link
Contributor

@dotanrs dotanrs commented Jul 12, 2020

This fixes the display of list items, which would still be aligned to the left despite being in dir=rtl

index.js Outdated
Comment on lines 5 to 10
items.forEach((item) => {
if (isRTL(item.innerText)) {
item.style['text-align'] = 'right';
}
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello @dotanrs

Thanks for your contribution and consideration here.
What do you think of using text-align: start; instead?

This should allow us to avoid checking if it's RTL/LTR. This is in line with the approach that we are already using with direction (auto, let the browser handle it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I love it. Changed it

Copy link
Owner

@obahareth obahareth left a comment

Choose a reason for hiding this comment

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

@dotanrs Looks good to me, can we just match the rest of the code and remove the semicolons? I'll setup a script in package.json to use Prettier to automatically do that and lint future PRs.

Thank you so much for finding and fixing this issue!

@dotanrs
Copy link
Contributor Author

dotanrs commented Jul 22, 2020

@obahareth sure thing, semicolons are removed 👍

@obahareth
Copy link
Owner

Awesome @dotanrs, I'll push a new version later in my evening today.

@obahareth obahareth merged commit 156dc00 into obahareth:master Jul 22, 2020
@obahareth
Copy link
Owner

@all-contributors please add @dotanrs for code contributions.

@allcontributors
Copy link
Contributor

@obahareth

I've put up a pull request to add @dotanrs! 🎉

@obahareth
Copy link
Owner

@all-contributors please add @EhsanZ for review contributions.

@allcontributors
Copy link
Contributor

@obahareth

I've put up a pull request to add @EhsanZ! 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants