search

servo / stylo

39 stars 11 forks source link

Add support for `start`, `end`, and `space-evenly` values to `align-` and `justify-content` properties #21

Closed nicoburns closed 2 months ago

nicoburns commented 3 months ago

Adds the start, end and space-evenly values to the align-content and justify-content enums. I will post a Servo PR implementing support for these values in layout2020's flexbox implementation shortly.

Corresponding servo PR: https://github.com/servo/servo/pull/31724

I have made the simplest change and just added these to Servo's existing enum, but the change I would really like to make is to switch Servo over to Gecko's content alignment implementation. This adds support for parsing:

EDIT: While the self-start and self-end values are representable by the underlying AlignFlags type, they are not parsed by the ContentDistribution wrapper.

Servo doesn't support all of those yet, but it also doesn't support stretch but still parses it, and I feel like it would be good to try to reduce the differences between the Servo and Gecko builds of Stylo.

P.S. Should I also be submitting this upstream to Mozilla (presumably only once it's accepted here?)? Or is there some kind of sync process that does this automatically?

mrobinson commented 3 months ago

Regarding the question from the PR description:

P.S. Should I also be submitting this upstream to Mozilla (presumably only once it's accepted here?)? Or is there some kind of sync process that does this automatically?

In this case the right repository to submit this to is this one. This is the sort of change we plan to upstream to Gecko once we get into sync. The reasoning is that we need this change for Servo.

nicoburns commented 3 months ago

@mrobinson I've rebased this on top of the latest main. I think we probably will want to switch to the Gecko implementation, and I am able to work on that if there is consensus. But my preference would be to land this first and then proceed with that as a separate change. Partly because it is already implemented (and it's such a tiny change), and partly because that allows us to separate algorithmic changes from type refactoring on the Servo side, and there may well be some other properties we want to switch over at the same time (e.g. align-items/justify-items).

P.S. How does synchronising landing changes here and in the servo repo work? Land here first and then update the Servo PR to point to the new commit? (Is Servo relying on Cargo.lock to pin a specific Stylo commit? - and is there a reason why it's not pinning a specific commit in Cargo.toml?)