WB-1655: Add mediaQuery breakpoints to wonder-blocks-tokens #2349
Conversation
🦋 Changeset detectedLatest commit: 9733fc0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
|
Size Change: +259 B (+0.26%) Total Size: 101 kB
ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-hmpwdvfyty.chromatic.com/ Chromatic results:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2349 +/- ##
============================
============================
Continue to review full report in Codecov by Sentry.
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (2f1741e) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2349"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2349 |
| const mediaQuery = { | ||
| // Named |
There was a problem hiding this comment.
There should probably be a comment here telling people if they make changes here they need to also update aphrodite.d.ts.
| const xs = | ||
| "@media screen and (max-width: 567px) /* breakpoint.mediaQuery.xs */"; | ||
| const sm = | ||
| "@media screen and (min-width: 568px) and (max-width: 681px) /* breakpoint.mediaQuery.sm */"; |
There was a problem hiding this comment.
There should probably be a comment here telling people if they make changes here they also need to update tokens/media-queries.ts.
@kevinb-khan good idea. Do you know of an example of that being done in Wonder Blocks? Could the media-queries.ts file be the source of truth to generate the type definition, or would it have to be the other way around? Some of this will need to change when we move away from Aphrodite, anyway. |
|
|
||
| const mediaQuery = { | ||
| // Named | ||
| xs: `@media screen and (max-width: ${width.xsMax}px) /* breakpoint.mediaQuery.xs */`, |
There was a problem hiding this comment.
Is /* breakpoint.mediaQuery.xs */ useful for IDE auto completion or something else?
I was looking at the Docs and that info appears duplicated in the table.
Again, if this is helpful for DX reasons, you can surely leave it as it is.
There was a problem hiding this comment.
Good question! It was a trick to avoid conflicts for typing of strings. I included it based on this comment in webapp from @kevinb-khan: https://github.com/Khan/webapp/blob/55c4465017a829d53bdc6d00d94d6d93e3b5dfe4/services/static/javascript/shared-styles-package/media-queries.ts#L26
We attach a unique comment string to each of the media queries in this file. That way, if another media query has the same constraints as one of these queries, their strings won't be identical, and they therefore won't conflict.
Admittedly, the typing for this was really hard to get right and I still wasn't that happy with it in the end. It would be one of the first things I'd want to refactor for the Aphrodite successor.
Co-authored-by: Bea Esguerra <beaesguerra@users.noreply.github.com>
Adds initial media query tokens for breakpoint sizes in wonder-blocks-tokens. Breakpoint values are derived from internal docs:
The structure of this new
mediaQueriesobject mimics the version in webapp (internal) pretty closely, but with values documented internally in Confluence: (see "Future Breakpoints"): https://khanacademy.atlassian.net/wiki/spaces/WB/pages/2099970518/Layout+BreakpointsScreenshot of the new docs, note there aren't examples of each one like the other tokens:
I did some initial testing in Wonder Blocks to ensure the type definition works and paired with @kevinbarabash to see if we could make it any more dynamic. But I think simple is fine for now as it's a bit of dark magic how these media query fragments turn into CSS Properties in Aphrodite.
My next step will be to update all
@mediaviewport sizing in WB to use these new breakpoints!Jira issue: https://khanacademy.atlassian.net/browse/WB-1655