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

Implementing Box Sizing for Stylo #11743

Merged
merged 1 commit into from Jun 23, 2016
Merged

Conversation

@shinglyu
Copy link
Member

shinglyu commented Jun 14, 2016

Implementing box-sizing for Stylo

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@shinglyu
Copy link
Member Author

shinglyu commented Jun 14, 2016

r? @emilio

@highfive highfive assigned emilio and unassigned pcwalton Jun 14, 2016
@shinglyu
Copy link
Member Author

shinglyu commented Jun 14, 2016

@emilio Can we add test to it or it's hard to test?

@@ -380,7 +380,6 @@ impl Debug for ${style_struct.gecko_struct_name} {
# These have unusual representations in gecko.
force_stub += ["list-style-type", "text-overflow"]
# Enum class instead of NS_STYLE_...

This comment has been minimized.

@Ms2ger

Ms2ger Jun 14, 2016

Contributor

Remove the comment?

}
}

fn copy_box_sizing_from(&mut self, other: &Self) {

This comment has been minimized.

@emilio

emilio Jun 14, 2016

Member

This could probably use the impl_simple_copy helper, though it's not a huge blocker.

r=me with @Ms2ger's comment addressed and (preferably, but optionally) this :)

use gecko_bindings::structs::StyleBoxSizing;
self.gecko.mBoxSizing = match v {
T::content_box => StyleBoxSizing::Content,
T::border_box => StyleBoxSizing::Border

This comment has been minimized.

@emilio

emilio Jun 14, 2016

Member

Also, please add a note here about padding-box, something like:

// TODO: guess what to do with box-sizing: padding-box

Thanks for your contribution! :)

@shinglyu shinglyu force-pushed the shinglyu:stylo-box-sizing branch from 7f159e4 to 9b2db6c Jun 15, 2016
T::border_box => StyleBoxSizing::Border
}
}
<%call expr="impl_simple_copy('box_sizing', 'mBoxSizing')"></%call>

This comment has been minimized.

@emilio

emilio Jun 17, 2016

Member

Can't this just be ${impl_simple_copy('box_sizing', 'mBoxSizing')}.

If not, it's fine to merge :)

@shinglyu shinglyu force-pushed the shinglyu:stylo-box-sizing branch from 9b2db6c to 8f97c89 Jun 17, 2016
@emilio
Copy link
Member

emilio commented Jun 23, 2016

Cool, thanks for doing this @shinglyu!

@bors-servo: r+

@bors-servo
Copy link
Contributor

bors-servo commented Jun 23, 2016

📌 Commit 8f97c89 has been approved by emilio

@bors-servo
Copy link
Contributor

bors-servo commented Jun 23, 2016

Testing commit 8f97c89 with merge c335bd7...

bors-servo added a commit that referenced this pull request Jun 23, 2016
Implementing Box Sizing for Stylo

<!-- Please describe your changes on the following line: -->
Implementing box-sizing for Stylo
---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] 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/11743)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 23, 2016

💔 Test failed - linux-rel

@jdm
Copy link
Member

jdm commented Jun 23, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Jun 23, 2016

Testing commit 8f97c89 with merge c04d245...

bors-servo added a commit that referenced this pull request Jun 23, 2016
Implementing Box Sizing for Stylo

<!-- Please describe your changes on the following line: -->
Implementing box-sizing for Stylo
---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] 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/11743)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 23, 2016

@bors-servo bors-servo merged commit 8f97c89 into servo:master Jun 23, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.