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

Support Overflow::Scroll #446

Merged
merged 17 commits into from
Apr 23, 2023

Conversation

nicoburns
Copy link
Collaborator

@nicoburns nicoburns commented Apr 16, 2023

Objective

  • To make it easier to use Taffy for UI trees containing scrollviews on systems that have scrollbars that take up space in the layout.
  • To support layouts containing overflow: scroll for those wanting to faithfully render web layouts.

Tasks

  • Add style types
  • Implement for leaf algorithm
  • Implement for flexbox algorithm
  • Implement for grid algorithm

Context

Feedback wanted

  • scrollbar_width deviates from the spec. The spec only allow auto, thin or none and the user agent must decide what actual widths these correspond to. Whereas I've allowed a pixel value to be specified. I think this difference is justified because Taffy is aimed at "user agents" rather than end users.
  • I've made scrollbar_width a u8 to save on space. Does this seem reasonable, or do you think I should make it an f32 like everything else? scrollbar_width has been converted to f32 in response to feedback.
  • I've used a single scrollbar_width for both axis (the overflow property still applies separately in each axis). So IF there is a scrollbar in both axis then they must both be the same size, does this seem reasonable.

@nicoburns nicoburns added enhancement New feature or request breaking-change A change that breaks our public interface labels Apr 16, 2023
@alice-i-cecile
Copy link
Collaborator

scrollbar_width deviates from the spec. The spec only allow auto, thin or none and the user agent must decide what actual widths these correspond to. Whereas I've allowed a pixel value to be specified. I think this difference is justified because Taffy is aimed at "user agents" rather than end users.

I agree: that's an unhelpful spec setup.

I've made scrollbar_width a u8 to save on space. Does this seem reasonable, or do you think I should make it an f32 like everything else?

I think we should probably be consistent rather than clever here. I can't imagine this making a performance difference, and fractional sizes are reasonable.

I've used a single scrollbar_width for both axis (the overflow property still applies separately in each axis). So IF there is a scrollbar in both axis then they must both be the same size, does this seem reasonable.

That seems sensible: uneven scroll bars definitely seem wrong.

@nicoburns nicoburns marked this pull request as ready for review April 23, 2023 21:47
@nicoburns nicoburns changed the title WIP: Support Overflow::Scroll Support Overflow::Scroll Apr 23, 2023
Copy link
Collaborator

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Good docs, clean code. Great tests as always.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change A change that breaks our public interface enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants