Skip to content
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

Repair and refine embed block styles #21599

Merged
merged 3 commits into from
May 13, 2020

Conversation

jasmussen
Copy link
Contributor

This is a rebased version of #14648, so props to @m-e-h.

It's a good PR that aims to improve and refine the embed CSS to be less specific and work better. It also fixes some code quality and is overall a good PR.

Thank you @m-e-h for the PR!

@jasmussen jasmussen added [Type] Code Quality Issues or PRs that relate to code quality [Block] Embed Affects the Embed Block labels Apr 15, 2020
@github-actions
Copy link

github-actions bot commented Apr 15, 2020

Size Change: -86 B (0%)

Total Size: 824 kB

Filename Size Change
build/block-library/style-rtl.css 7.32 kB -43 B (0%)
build/block-library/style.css 7.33 kB -43 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.61 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-editor/index.js 102 kB 0 B
build/block-editor/style-rtl.css 10.3 kB 0 B
build/block-editor/style.css 10.3 kB 0 B
build/block-library/editor-rtl.css 7.12 kB 0 B
build/block-library/editor.css 7.12 kB 0 B
build/block-library/index.js 115 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 180 kB 0 B
build/components/style-rtl.css 17 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.45 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 4.41 kB 0 B
build/edit-navigation/style-rtl.css 618 B 0 B
build/edit-navigation/style.css 617 B 0 B
build/edit-post/index.js 28 kB 0 B
build/edit-post/style-rtl.css 12.2 kB 0 B
build/edit-post/style.css 12.2 kB 0 B
build/edit-site/index.js 12.1 kB 0 B
build/edit-site/style-rtl.css 5.22 kB 0 B
build/edit-site/style.css 5.22 kB 0 B
build/edit-widgets/index.js 8.37 kB 0 B
build/edit-widgets/style-rtl.css 4.69 kB 0 B
build/edit-widgets/style.css 4.69 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/index.js 44.3 kB 0 B
build/editor/style-rtl.css 5.07 kB 0 B
build/editor/style.css 5.08 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.63 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.14 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@jasmussen jasmussen force-pushed the fix/embed-block-styles-repair-refine branch from f8c0d75 to be5f8ba Compare May 11, 2020 08:02
@jasmussen jasmussen added the [Status] In Progress Tracking issues with work in progress label May 11, 2020
@jasmussen jasmussen force-pushed the fix/embed-block-styles-repair-refine branch 2 times, most recently from 621c8c4 to e82f93e Compare May 11, 2020 08:27
@jasmussen jasmussen removed the [Status] In Progress Tracking issues with work in progress label May 11, 2020
@jasmussen
Copy link
Contributor Author

Okay, rebased, refreshed, and tested again. The embed block with responsiveness works as it should, but this still makes for better specificity. It also fixes a few typos, and it fixes an issue where a floated embed can collapse into nothingness:

Screenshot 2020-05-11 at 09 57 31

Screenshot 2020-05-11 at 10 20 35

@jasmussen jasmussen force-pushed the fix/embed-block-styles-repair-refine branch from e82f93e to 46b03bd Compare May 11, 2020 11:14
@jasmussen jasmussen force-pushed the fix/embed-block-styles-repair-refine branch from 46b03bd to 91aff35 Compare May 11, 2020 12:28
@ZebulanStanphill
Copy link
Member

I tried to test this PR on gutenberg.run, but trying to embed anything keeps failing there.

Just looking at the code changes, though, they seem like an improvement to me.

Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally. LGTM!

@jasmussen
Copy link
Contributor Author

THANK YOU!

Comment on lines -72 to +77
&.wp-embed-aspect-9-16 .wp-block-embed__wrapper::before {
padding-top: 177.78%; // 16 / 9 * 100
}
.wp-embed-aspect-9-6 .wp-block-embed__wrapper::before {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasmussen This appears to have been typo, to provide 6 instead of 16. Fix provided in #25972.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Embed Affects the Embed Block [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants