-
Notifications
You must be signed in to change notification settings - Fork 3.4k
update(docs, layout): fix breakpoints, documentation, and gettingStarted #6082
Conversation
* update component custom mediaQueries to be consistent with ranges defined for Layout mediaQueries * update `hide` API for mediaQueries * fix hide SCSS and component mediaQuery(s) * fix mixins for gt-lg and xl * fix flex-order with negative ordering; support -20 to 20 Fixes #5646. Fixes #6056. Fixes #5576.
@jelbourn, @robertmesserle, @EladBezalel, @topherfangio - please review ASAP as I want to push to master this morning... so @naomiblack can review and word-smith. |
docs/app/js/app.js
Outdated
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.
Since you've renamed, perhaps we don't need the comment 😄
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.
Possibly reword: "watch the videos about the Angular.js framework".
Also, why is the title for both of these "Link opens in a new window"? Shouldn't it be related to the link? (I realize this was not your change, just wondering if there is a reason this was done.)
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.
Agree with Topher.
- Make the entire sentence a hyperlink
- "Angular.js" > "Angular"
- Update
title
to say what's behind the link
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.
My interpretation is that when the user hovers over the link, it will show this tip of "intended action".
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.
...The title attribute is used to provide additional information to help clarify or further describe the purpose of a link. If the supplementary information provided through the title attribute is something the user should know before following the link, such as a warning, then it should be provided in the link text rather than in the title attribute.
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.
Extra single quote and space ('
) at the end of the link.
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.
LOVE this addition; hopefully it will help people when creating issues too...
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.
We should be hosting this image, not githubusercontent. Should also run the image through pngcrush
(I can do this from my workstation if that's easier).
The images also need alt
text.
Currently `validateAttributeUsage( )` is only used for the `flex` API, so remove its use from the non-observable APIs
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.
Typo, add is
: "Note that <code>row</code>
is the default layout direction if..."
LGTM, will review this locally later. |
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.
Typo: "while and"
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.
Weird indentation
@ThomasBurleson Finished a content review after reading through each of the layout docs. Are there any functionality changes you would like us to test as well? |
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 section is worded like you're in the middle of a step-by-step tutorial rather than API docs. Instead something like
"The breakpoint alias combines with the Layout API, allowing..."
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.
True... the goal is full clarity of how the sugar works and why.
wrapper div for `<p>` tags min-height for `<div class="menus">` Fixes #5962.
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 Layout API means that developers no longer have to worry about the details of FlexBox styling."
This is like saying GWT means that developers no longer have to worry about JavaScript. I would reword this to say that it makes it much easier to set up and maintain flexbox layouts vs. defining everything via CSS.
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.
The Layout engine will even report console warnings for Layout conflicts and issues that are not automatically resolved.
"The Layout engine will log console warnings when it encounters conflicts or known issues."
gt-lg
andxl
hide
API / mediaQuerieshide
andshow
SCSS and component mediaQuery(s)flex-order
with negative ordering; support -20 to 20Fixes #4895. Fixes #5646. Fixes #6056. Fixes #5576.