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

Implement 'vertical-align' property. #741

Closed
wants to merge 2 commits into from
Closed

Implement 'vertical-align' property. #741

wants to merge 2 commits into from

Conversation

@june0cho
Copy link
Contributor

june0cho commented Aug 19, 2013

It implements 'vertical-align' property and adds a simple test case.

However, there are several things to be fixed.

  • We should calculate top and bottom value for margin, border, and padding in advance.
  • For the 'middle' value of vertical-align, we should know 'x-height' of font.
  • For the 'sub' and 'super' value of vertical-align, the proper position should be chosen.
june0cho added 2 commits Aug 14, 2013
@june0cho
Implement 'vertical-align' property
b20565c
@june0cho
Add a simple test case for 'vertical-align'
29371d8
@june0cho
Copy link
Contributor Author

june0cho commented Aug 20, 2013

I'm modifying this code to fix some problems and support 'line-height'. I'm closing this PR and will open new PR later.
@june0cho june0cho closed this Aug 20, 2013
@eric93
Copy link

eric93 commented Aug 20, 2013

One thing I was curious about: what are adjust and adjust_flag used for?
@june0cho
Copy link
Contributor Author

june0cho commented Aug 20, 2013

adjust is used when positions of all boxes need to be moved. For example, if offset becomes less than 0, all boxes' positions should be moved down the same size. (But I found out that the current code has a problem which ignores line-height. This is why 'line-height' doesn't work.)

When I try to find minimum offset among all boxes, I used adjust_flag in order to just set initial value.
@june0cho june0cho mentioned this pull request Aug 23, 2013
Closed
bors-servo pushed a commit that referenced this pull request Aug 27, 2013
bors-servo
auto merge of #764 : june0cho/servo/verticalalign, r=metajack
4e68c1a 
This is a modification of the previous PR #741 to support 'vertical-align' and 'line-height'. Added test cases for vertical-align, line-height, and mixed.

In the last commit, after applying 'vertical-align' property, the current line-box's height and following line-boxes' heights are being updated. This is because line-box's height was already assigned when the line-box is created. 
But there may be a better way, though I'm not sure. In Gecko, line-boxes' heights seem to be assigned at one time after 'vertical-align'. @metajack
ChrisParis pushed a commit to ChrisParis/servo that referenced this pull request Sep 7, 2014
@hayatoito
Merge pull request servo#741 from KenjiBaheux/remove-pseudo-element
3b2c398 
Removed tests using pseudo element
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
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@june0cho @eric93
You can’t perform that action at this time.