Skip to content

Conversation

@ockham
Copy link
Contributor

@ockham ockham commented Oct 13, 2020

Description

Found while working on #26066 with @sirreal.

<span /> doesn't seem to actually accept width and height props: It is an inline element, and its attributes are the global attributes which do not include width/height.

SO thread with some more info: https://stackoverflow.com/questions/2491068/does-height-and-width-not-apply-to-span

Thus, this PR removes those props, and the size prop that's used to set them.

How has this been tested?

Do some smoke testing to verify that dashicons still work.

Furthermore, verify that unit tests pass:

npm run test-unit -- packages/components/src/icon/

Types of changes

Bug fix/HTML standards compliance fix.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@ockham ockham added the [Package] Components /packages/components label Oct 13, 2020
@ockham ockham self-assigned this Oct 13, 2020
@github-actions
Copy link

github-actions bot commented Oct 13, 2020

Size Change: -19 B (0%)

Total Size: 1.19 MB

Filename Size Change
build/components/index.js 170 kB -19 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.54 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 668 B 0 B
build/block-directory/index.js 8.61 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 8.93 kB 0 B
build/block-library/editor.css 8.93 kB 0 B
build/block-library/index.js 144 kB 0 B
build/block-library/style-rtl.css 7.71 kB 0 B
build/block-library/style.css 7.71 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.77 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.6 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.63 kB 0 B
build/core-data/index.js 12.1 kB 0 B
build/data-controls/index.js 683 B 0 B
build/data/index.js 8.63 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 4.43 kB 0 B
build/edit-navigation/index.js 10.6 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.37 kB 0 B
build/edit-post/style.css 6.35 kB 0 B
build/edit-site/index.js 21.6 kB 0 B
build/edit-site/style-rtl.css 3.8 kB 0 B
build/edit-site/style.css 3.81 kB 0 B
build/edit-widgets/index.js 22.3 kB 0 B
build/edit-widgets/style-rtl.css 3.09 kB 0 B
build/edit-widgets/style.css 3.09 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 42.6 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.38 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.35 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.61 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.75 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@sirreal
Copy link
Member

sirreal commented Oct 14, 2020

Rebased to pull in test fixes.

@sirreal sirreal force-pushed the remove/dashicon-size-prop branch 2 times, most recently from 42293f2 to 43f9ab4 Compare October 14, 2020 09:10
@ockham ockham force-pushed the remove/dashicon-size-prop branch from 43f9ab4 to 09a20d4 Compare October 14, 2020 15:29
@ockham
Copy link
Contributor Author

ockham commented Oct 14, 2020

Rebased after merging #26066.

@ockham ockham force-pushed the remove/dashicon-size-prop branch from 09a20d4 to cbbc4c8 Compare October 19, 2020 09:53
@ockham ockham marked this pull request as ready for review October 19, 2020 10:19
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

It makes sense.

@ockham ockham merged commit 717a6c4 into master Oct 19, 2020
@ockham ockham deleted the remove/dashicon-size-prop branch October 19, 2020 14:10
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Components /packages/components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants