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

Add transitions for CSS `calc()`. #7653

Merged
merged 1 commit into from Sep 20, 2015
Merged

Add transitions for CSS `calc()`. #7653

merged 1 commit into from Sep 20, 2015

Conversation

@notriddle
Copy link
Contributor

notriddle commented Sep 17, 2015

Closes #7284

Review on Reviewable
@notriddle
Copy link
Contributor Author

notriddle commented Sep 17, 2015

r? @dzbarsky
@dzbarsky
Copy link
Member

dzbarsky commented Sep 17, 2015

I'm surprised by the conversion of 100px to calc(100px + 0%). What do other browsers do here?

As I remember the calc spec, they mentioned keeping values as calc even with a component of 0 would avoid issues with the function not interpolating smoothly, which would seem to imply that this should use the from state for 0-50%, and then switch to the to state.
@notriddle
Copy link
Contributor Author

notriddle commented Sep 17, 2015

The main reason for that conversation is to support stuff like transitioning from 100px to 50%. The spec days that should work the same as transitioning from calc(100px + 0%) to calc(0px + 50%).
@notriddle notriddle closed this Sep 17, 2015
@notriddle notriddle reopened this Sep 17, 2015
@notriddle
Copy link
Contributor Author

notriddle commented Sep 17, 2015

Wrong button...
@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2015

☔ The latest upstream changes (presumably #7662) made this pull request unmergeable. Please resolve the merge conflicts.
@notriddle
Add transitions for CSS `calc()`.
Loading status checks…
554a4cf 
Closes #7284
@dzbarsky
Copy link
Member

dzbarsky commented Sep 20, 2015

Ok, I checked and it looks like browsers actually do this conversion thing. r=me
@dzbarsky
Copy link
Member

dzbarsky commented Sep 20, 2015

I don't think bors listens to me so you'll have to get someone to trigger the merge.
@jdm
Copy link
Member

jdm commented Sep 20, 2015

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

bors-servo commented Sep 20, 2015

📌 Commit 554a4cf has been approved by dzbarsky
@bors-servo
Copy link
Contributor

bors-servo commented Sep 20, 2015

⌛ Testing commit 554a4cf with merge 5667283...
bors-servo pushed a commit that referenced this pull request Sep 20, 2015
bors-servo
Auto merge of #7653 - notriddle:calc-transition, r=dzbarsky
Loading status checks…
5667283 
Add transitions for CSS `calc()`.

Closes #7284

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

bors-servo commented Sep 20, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit 554a4cf into servo:master Sep 20, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@notriddle notriddle deleted the notriddle:calc-transition branch Oct 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@notriddle @dzbarsky @bors-servo @jdm @highfive
You can’t perform that action at this time.