-
Notifications
You must be signed in to change notification settings - Fork 524
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(interactive-examples): support HTML examples #12633
Conversation
Bundle StatsHey there, this message comes from a github action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
View detailed bundle breakdownAdded
Removed
Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
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.
Initial review pass, didn't test yet.
@LeoMcA Should I replicate these tests to verify? If so, can you please share instructions? |
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.
Claas left a few comments to discuss already, otherwise this looks very good!
background: var(--background-secondary); | ||
border-bottom: 1px solid var(--border-secondary); | ||
display: flex; | ||
flex-shrink: 0; |
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.
Nit: Since flex-grow
defaults to 0 (unless set elsewhere I don't see), we can use the short-hand for clarity (!?).
flex-shrink: 0; | |
flex: 0; |
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 it would have to be flex: 0 0;
, and at least to me using flex-shrink directly is clearer: I think "this flex item can't shrink" rather than "wait what order are the arguments in flex again"
Note
Look at second commit onwards for easier diff.
Summary
https://github.com/orgs/mdn/discussions/782
Adds support for HTML examples, including:
How did you test this change?
Another round of snapshot image diff testing to do, alongside rari diff testing.