-
Notifications
You must be signed in to change notification settings - Fork 516
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
Issue 561 Added example for vertical-align in text context #606
Conversation
💖 Thanks for opening this pull request! 💖 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this works great :).
I pushed a merge from the latest master, because the master you started from had a problem which stopped the example from working.
Apart from that, the only thing I can see is that this example doesn't belong in the "text" directory. We are trying to organize CSS examples by the specification that defines them, and I think "inline-layout" would be the right directory for this property, mapping to https://drafts.csswg.org/css-inline/.
…audet/interactive-examples into issue-561-css-vertical-align
Hello @wbamberg, |
This looks fine! Just you forgot to update the paths inside meta.json when you moved the files from "text" to "inline-layout". I just pushed a commit taking care of that. I also made the CSS selector more specific - as you pointed out before, it is possible for styles in here to interfere with the whole example - they don't in this particular case, but still it seems like a good idea to avoid that possibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @LLyaudet !
Congrats on merging your first pull request! 🎉🎉🎉 |
I've just updated the page: https://developer.mozilla.org/en-US/docs/Web/CSS/vertical-align :) |
Hello @wbamberg,
Here is the pull request for vertical-align. I put my commit in a branch issue-561-css-vertical-align if there is any correction to add to the branch.
Best regards,
Laurent Lyaudet