Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upPartial flex implementation of flex-grow, flex-shrink and flex-basis for inline mode #12298
Conversation
highfive
commented
Jul 6, 2016
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @SimonSapin (or someone else) soon. |
components/layout/flex.rs
Outdated
| // TODO shrink based on scaled_shrink_factor | ||
| let percent_shrink = kid.flow.as_block().fragment.style.get_position().flex_shrink / shrink_count; | ||
| Au::to_f32_px(grow_space_available) * percent_shrink | ||
| }else if grow_space_available > Au(0) && grow_count > 0_f32 { |
This comment has been minimized.
This comment has been minimized.
components/layout/flex.rs
Outdated
| }else if grow_space_available > Au(0) && grow_count > 0_f32 { | ||
| let percent_grow = kid.flow.as_block().fragment.style.get_position().flex_grow / grow_count; | ||
| Au::to_f32_px(grow_space_available) * percent_grow | ||
| }else { |
This comment has been minimized.
This comment has been minimized.
components/layout/flex.rs
Outdated
| } | ||
|
|
||
| impl FlexItem { | ||
| fn new(flow: FlowRef) -> FlexItem { | ||
| FlexItem { | ||
| flow: flow | ||
| flow: flow, basis: Au(0) |
This comment has been minimized.
This comment has been minimized.
components/layout/flex.rs
Outdated
|
|
||
| base.block_container_inline_size = even_content_inline_size; | ||
| let base = flow::mut_base(flow_ref::deref_mut(&mut kid.flow)); |
This comment has been minimized.
This comment has been minimized.
pcwalton
Jul 6, 2016
Contributor
Hmm, I'm not a big fan of pushing things down instead of grabbing from the parent, but the old code did this too, so this is OK.
This comment has been minimized.
This comment has been minimized.
stshine
Jul 6, 2016
Contributor
How would you suggest to exactly do that? IMO there is no way for a flow child to get a pointer to its parent.
This comment has been minimized.
This comment has been minimized.
…for inline mode.
components/layout/flow.rs
Outdated
| @@ -679,6 +679,8 @@ bitflags! { | |||
|
|
|||
| /// Whether this flow contains any text and/or replaced fragments. | |||
| const CONTAINS_TEXT_OR_REPLACED_FRAGMENTS = 0b0001_0000_0000_0000_0000_0000, | |||
|
|
|||
| const IS_FLEX = 0b0010_0000_0000_0000_0000_0000, | |||
| } | |||
This comment has been minimized.
This comment has been minimized.
stshine
Jul 6, 2016
Contributor
This should be done in block flags since block is the only child type of a flex container.
|
Hmm... Currently this PR does not fix #12191 because these properties are marked as experimental and not enabled. If you want them to be enabled in normal run you should add them to |
|
This looks good to me. Is this ready to go? |
|
(tracking #12453) |
Closed
|
@bors-servo: r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Jul 18, 2016
Partial flex implementation of flex-grow, flex-shrink and flex-basis for inline mode <!-- Please describe your changes on the following line: --> Implemented flex-grow, flex-shrink, and flex-basis for the case where flex-direction is row or row-reverse. Prior to this change elements would not actually flex. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #12191 <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12298) <!-- Reviewable:end -->
|
|
|
|
|
|
This can be closed since #12330 is merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
schmidek commentedJul 6, 2016
•
edited by izgzhen
Implemented flex-grow, flex-shrink, and flex-basis for the case where flex-direction is row or row-reverse. Prior to this change elements would not actually flex.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is