-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
feat(layout): add window breakpoints from spec #9318
Conversation
src/cdk/layout/breakpoints.ts
Outdated
WindowSmall: '(min-width: 600px) and (max-width: 959px)', | ||
WindowMedium: '(min-width: 960px) and (max-width: 1279px)', | ||
WindowLarge: '(min-width: 1280px) and (max-width: 1919px)', | ||
WindowXLarge: '(min-width: 1920px)', |
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.
@josephperrott I looked at the spec and it's not clear to me what "Window" even means in that table. Is it supposed be be a "desktop browser window"?
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 believe so. That mostly is coming from listing breakpoints "across desktop, mobile and tablet". And mobile (written elsewhere as handset) and tablet are referenced so the others must be desktop?
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.
Would it make sense to make these DesktopSmall
, DesktopMedium
, etc.?
It would also be nice to confirm with the Material Design team.
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.
After some discussion, @jelbourn and I determined that we think the best option would be to drop the prefix of the sizes entirely.
XSmall: '(max-width: 599px)',
Small: '(min-width: 600px) and (max-width: 959px)',
Medium: '(min-width: 960px) and (max-width: 1279px)',
Large: '(min-width: 1280px) and (max-width: 1919px)',
XLarge: '(min-width: 1920px)',
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.
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.
LGTM
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #9212