Conversation
|
To answer your questions:
left should mean left, etc.
My intuition on this is that it should happen during the calculations. But I would be open to being persuaded otherwise. As long as whichever implementation we choose is able to pass all the relevant tests and match other browsers I think I'll be happy. Will do a proper review later. |
nicoburns
left a comment
There was a problem hiding this comment.
@tjamaan Actual direction changes look good to me. But we'll want to hook this up to our testing infrastructure to verify that. The process for that is something like:
- Add support for the
directionproperty to ourgentestscript- Add support for reading it to
scripts/gentest/test_helper.js - Add support for generating to
scripts/gentest/main.rs
- Add support for reading it to
- Setup the gentest script to generate every test with both
ltrandrtlas the default. - Write some new tests to test mixed direction.
I've also formatted main using the latest nightly, and I've also removed .cargo/config from our repo. So if you rebase on latest main that should get rid of a bunch of unrelated changes from the diff.
| /// TODO: documentation | ||
| pub direction: Direction, |
There was a problem hiding this comment.
Am I correct in thinking that this isn't actually used? If so, it should probably be removed. Removing the direction field here and the direction function parameter from the traits would greatly reduce the size of the diff for this PR which would be nice!
| match direction { | ||
| Direction::Rtl => { | ||
| justify_content.flip(); | ||
| justify_items.as_mut().map(|x| x.flip()); | ||
| } | ||
| _ => (), | ||
| } |
There was a problem hiding this comment.
Maybe use if direction.is_rtl() here?
|
@tjamaan Are you intending to finish this feature or should I mark this as open for adoption? One thing that has changed recently is that there is less need for writing custom tests as we are now able to run the WPT tests against Taffy using Blitz ( |
|
hey @nicoburns I'd like to adopt this PR! Should I fork the repo and apply these commits or continue on this branch |
|
@kane50613 That's great! I think you'll need to fork and create you're own branch, as I don't think github will let you push tobthis one. |
diff is adpoted from DioxusLabs#736 originally written by @tjamaan
diff is adopted from DioxusLabs#736 originally written by @tjamaan
diff is adopted from DioxusLabs#736 originally written by @tjamaan
diff is adopted from DioxusLabs#736 originally written by @tjamaan add grid & flex test fixtures add block layout & inherited fixtures, remove unused `flip` function better `direction` field comments for consistency merge `alignment_based_offset` match cases remove newline Fix double application of inset Add edge helpers to ContentSlot Fixup x/y positioning of block children This fixes errors introduced when resolving merge conflicts when rebasing the floats PR on top of the direction PR. Fix build when block_layout is enabled bu float_layout isn't Signed-off-by: Nico Burns <nico@nicoburns.com> Fix build when no default features are enabled Signed-off-by: Nico Burns <nico@nicoburns.com> remove redundant `direction` prop from layout inputs update test helper to get direction from computed style Account for the possibility of negative free space Don't account for margins when computing slot width Don't add scrollbar_width to RTL x position of flex items The scrollbar of items is within their width. correct RTL grid placement cargo fmt Signed-off-by: Nico Burns <nico@nicoburns.com> Create rtl variants of all tests Signed-off-by: Nico Burns <nico@nicoburns.com> fix fix `TrackCounts` use add `AlignContent::reversed`, avoid using `ContentSlot` without `float_layout` guard remove `scrollbar_size` redeclartion simplify changes simplify `rtl_column_occupancy_index_for_initialization` params reduce diff cargo fmt simplify flexbox rtl offset math fix `round` on negative value README: add slint (DioxusLabs#918)
* implement `direction` property diff is adopted from #736 originally written by @tjamaan add grid & flex test fixtures add block layout & inherited fixtures, remove unused `flip` function better `direction` field comments for consistency merge `alignment_based_offset` match cases remove newline Fix double application of inset Add edge helpers to ContentSlot Fixup x/y positioning of block children This fixes errors introduced when resolving merge conflicts when rebasing the floats PR on top of the direction PR. Fix build when block_layout is enabled bu float_layout isn't Signed-off-by: Nico Burns <nico@nicoburns.com> Fix build when no default features are enabled Signed-off-by: Nico Burns <nico@nicoburns.com> remove redundant `direction` prop from layout inputs update test helper to get direction from computed style Account for the possibility of negative free space Don't account for margins when computing slot width Don't add scrollbar_width to RTL x position of flex items The scrollbar of items is within their width. correct RTL grid placement cargo fmt Signed-off-by: Nico Burns <nico@nicoburns.com> Create rtl variants of all tests Signed-off-by: Nico Burns <nico@nicoburns.com> fix fix `TrackCounts` use add `AlignContent::reversed`, avoid using `ContentSlot` without `float_layout` guard remove `scrollbar_size` redeclartion simplify changes simplify `rtl_column_occupancy_index_for_initialization` params reduce diff cargo fmt simplify flexbox rtl offset math fix `round` on negative value README: add slint (#918) * Hide debug output in tests * Disable rounding for some tests * Update relayout_is_stable_with_rounding tests to new "round_ties_up" rounding strategy --------- Co-authored-by: Nico Burns <nico@nicoburns.com>
|
Superceded by #906 |
Objective
directionCSS style property.directionproperty (right-to-left layout) #213Context
This change attempts to support bidi layout only for horizontal layout direction, i.e. it doesn't support vertical content such as CJK or Mongolian vertical writing systems.
Feedback wanted
Should inset, margin, padding, and border
Rects be considered as absolute axis (left means left and right means right) or relative axis (left means layout start, right means layout end)?Should the RTL conversion happen during the calculations or as a separate phase (i.e. calculate as if LTR then flip the horizontal axis)?
General code review is also wanted to be more aligned with the rest of taffy's code :)