-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
Mobile Deprecation version section is too long #1240
Mobile Deprecation version section is too long #1240
Conversation
✅ Deploy Preview for ember-deprecations ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
@@ -581,7 +581,7 @@ p a:hover { | |||
width: 100%; | |||
|
|||
.ember-version-graphic { | |||
$height: 132px; | |||
$height: 161px; |
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.
This makes the ember-version-graphic a tad bit larger, so 3 version numbers could fit per column. I also think it looks a bit better with it larger
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 like your design! It fixes what looks like a visual bug. I think the HTML markup needs some fixes before this can be merged, to ensure that our HTML is valid and accessible. Let me know if you have any questions or want help. Thank you for your work on this!
@@ -36,18 +36,24 @@ | |||
<li class="item list-unstyled"> | |||
<EmberVersionGraphic @mascot="ember" /> | |||
<ul class="links"> | |||
<li class="list-unstyled" data-test-ember-1-link> | |||
<div class="row"> |
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 that a ul
must have li
as direct children, without divs in between, based on the "Permitted content" section of the ul
docs: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/ul#technical_summary
One way to achieve this layout without the divs, while still keeping this one continuous list, is something like https://codepen.io/srikarg/pen/kxwQXV
There are lots of options for how to write this CSS. Another example is with flexbox: https://stackoverflow.com/questions/42613350/how-can-i-create-multi-columns-from-a-single-unordered-list
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.
Got it thanks! I'll push up a few commits to address this when I get a chance
#1239
Before:
After:
There is now less empty space on the right side, and the proportions just look a bit better.