Skip to content

/ servo

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

layout: Implement `<table width>` and `<center>`. #5980

Merged
merged 1 commit into from May 11, 2015
Merged

layout: Implement `<table width>` and `<center>`. #5980

merged 1 commit into from May 11, 2015

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented May 8, 2015

Improves Hacker News.

r? @mbrubeck

Review on Reviewable
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented May 8, 2015

Critic review: https://critic.hoppipolla.co.uk/r/4933

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.
@jdm
Copy link
Member

jdm commented May 8, 2015

I feel like the changes that add servo_center here are missing.
@mbrubeck
Copy link
Contributor

mbrubeck commented May 8, 2015

servo_center is added in the define_text_align! macro in properties.rs.mako.
@mbrubeck
Copy link
Contributor

mbrubeck commented May 9, 2015

Reviewed files:

  • components/layout/block.rs @ r1
  • components/layout/inline.rs @ r1
  • components/style/properties.mako.rs @ r1
  • resources/presentational-hints.css @ r1
  • resources/quirks-mode.css @ r1
  • resources/servo.css @ r1
  • tests/ref/basic.list @ r1

Comments from the review on Reviewable.io
@bors-servo
Copy link
Contributor

bors-servo commented May 11, 2015

☔ The latest upstream changes (presumably #5997) made this pull request unmergeable. Please resolve the merge conflicts.
@mbrubeck
Copy link
Contributor

mbrubeck commented May 11, 2015

Reviewed files:

  • components/style/legacy.rs @ r1
  • tests/ref/servo_center_a.html @ r1
  • tests/ref/servo_center_ref.html @ r1
  • tests/ref/table_width_attribute_a.html @ r1
  • tests/ref/table_width_attribute_ref.html @ r1

Comments from the review on Reviewable.io
@mbrubeck
Copy link
Contributor

mbrubeck commented May 11, 2015

r+, needs rebase.
@pcwalton pcwalton force-pushed the pcwalton:table-width-and-center branch from 093705e to a2ec5b2 May 11, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented May 11, 2015

@bors-servo: r=mbrubeck
@bors-servo
Copy link
Contributor

bors-servo commented May 11, 2015

📌 Commit a2ec5b2 has been approved by mbrubeck
@bors-servo
Copy link
Contributor

bors-servo commented May 11, 2015

⌛ Testing commit a2ec5b2 with merge fd0b0a8...
bors-servo pushed a commit that referenced this pull request May 11, 2015
bors-servo
Auto merge of #5980 - pcwalton:table-width-and-center, r=mbrubeck
fd0b0a8 
Improves Hacker News.

r? @mbrubeck

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5980)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 11, 2015

💔 Test failed - mac1
@waddlesplash
Copy link

waddlesplash commented May 11, 2015 

Unexpected Results
==================

/html/rendering/non-replaced-elements/tables/table-width-150percent.html
------------------------------------------------------------------------
PASS expected FAIL [Parent]
@pcwalton
layout: Implement `<table width>` and `<center>`.
Loading status checks…
4b9cd4e 
Improves Hacker News.
@pcwalton pcwalton force-pushed the pcwalton:table-width-and-center branch from a2ec5b2 to 4b9cd4e May 11, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented May 11, 2015

@bors-servo: r=mbrubeck
@bors-servo
Copy link
Contributor

bors-servo commented May 11, 2015

📌 Commit 4b9cd4e has been approved by mbrubeck
bors-servo pushed a commit that referenced this pull request May 11, 2015
bors-servo
Auto merge of #5980 - pcwalton:table-width-and-center, r=mbrubeck
Loading status checks…
c1e15e8 
Improves Hacker News.

r? @mbrubeck

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5980)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 11, 2015

⌛ Testing commit 4b9cd4e with merge c1e15e8...
@bors-servo
Copy link
Contributor

bors-servo commented May 11, 2015

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2
@bors-servo bors-servo merged commit 4b9cd4e into servo:master May 11, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request May 11, 2015
Merged
@SimonSapin
Copy link
Member

SimonSapin commented May 13, 2015

Shouldn’t the -servo-center keyword only be allowed in UA stylesheets?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mbrubeck mbrubeck

Labels
S-awaiting-review
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
@pcwalton @hoppipolla-critic-bot @jdm @mbrubeck @bors-servo @waddlesplash @SimonSapin @highfive
You can’t perform that action at this time.