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

flexbox: initial attempt to add support for RTL direction #1225

Merged
merged 6 commits into from
Oct 21, 2020

Conversation

malnajdi
Copy link
Contributor

@malnajdi malnajdi commented Oct 3, 2020

I have added:

  1. 2 test cases for flexbox with direction RTL.
  2. modified the code which handles flexbox to add RTL aware code.

related #601
related #106

position_axis += free_space / len(line) / 2
elif justify_content == 'space-evenly':
position_axis += free_space / (len(line) + 1)
if box.style['direction'] == 'rtl':
Copy link
Member

Choose a reason for hiding this comment

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

The logical part here is good. A nice improvement would be to multiply free_space by -1 here if direction is rtl, it would help keeping only one block below instead of duplicating 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 have tried to add what you required but it breaks the flex items

weasyprint/layout/flex.py Outdated Show resolved Hide resolved
weasyprint/tests/test_layout/test_flex.py Show resolved Hide resolved
article.position_x + article.width)
assert div_1.position_x > div_2.position_x > div_3.position_x


@assert_no_logs
Copy link
Member

Choose a reason for hiding this comment

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

Adding tests here with rtl + column is a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -58,7 +58,9 @@ def test_flex_direction_row_rtl():
div_2.position_y ==
div_3.position_y ==
article.position_y)
assert div_1.position_x == article.width - div_1.padding_width()
assert (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@liZe please verify if this test is correct.

assert (
div_3.position_x + div_3.width ==
article.position_x + article.width)
assert div_3.position_x == article.position_x
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@liZe please verify if this test is correct.

@malnajdi malnajdi requested a review from liZe October 10, 2020 19:08
@liZe
Copy link
Member

liZe commented Oct 21, 2020

Thanks a lot! No problem, I’ll simplify the "if rtl" part of your PR in a future commit.

@liZe liZe merged commit f5f204e into Kozea:master Oct 21, 2020
liZe added a commit that referenced this pull request Oct 21, 2020
@grewn0uille grewn0uille added this to the 52 milestone Oct 29, 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.

3 participants