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

Primitives: Add types #21482

Merged
merged 11 commits into from
Apr 10, 2020
Merged

Primitives: Add types #21482

merged 11 commits into from
Apr 10, 2020

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Apr 8, 2020

Description

Add types to @wordpress/primitives.
Taking notes from #21248. Part of #18838.

How has this been tested?

npm run build:package-types

Types of changes

Add types.

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.

cc/ @sirreal

@ockham ockham added the [Status] In Progress Tracking issues with work in progress label Apr 8, 2020
@ockham ockham self-assigned this Apr 8, 2020
@github-actions
Copy link

github-actions bot commented Apr 8, 2020

Size Change: -12 B (0%)

Total Size: 903 kB

Filename Size Change
build/block-editor/index.js 104 kB -7 B (0%)
build/primitives/index.js 1.49 kB -5 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.01 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.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.22 kB 0 B
build/block-library/index.js 112 kB 0 B
build/block-library/style-rtl.css 7.15 kB 0 B
build/block-library/style.css 7.16 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 57.7 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.1 kB 0 B
build/data-controls/index.js 1.25 kB 0 B
build/data/index.js 8.43 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 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 3.1 kB 0 B
build/edit-navigation/style-rtl.css 279 B 0 B
build/edit-navigation/style.css 280 B 0 B
build/edit-post/index.js 93.5 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.4 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.53 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.73 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/index.js 43.6 kB 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.29 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.13 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 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.91 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.28 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.67 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.67 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.01 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.17 kB 0 B

compressed-size-action

@ockham ockham mentioned this pull request Apr 8, 2020
6 tasks
@ockham ockham mentioned this pull request Apr 8, 2020
6 tasks
@ockham ockham added [Status] Blocked Used to indicate that a current effort isn't able to move forward and removed [Status] In Progress Tracking issues with work in progress labels Apr 8, 2020
@ockham ockham removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Apr 10, 2020
@ockham ockham marked this pull request as ready for review April 10, 2020 11:05
@gziolo gziolo added the [Type] Code Quality Issues or PRs that relate to code quality label Apr 10, 2020
@ockham
Copy link
Contributor Author

ockham commented Apr 10, 2020

Snaphots have changed, I'll update.

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

Comment on lines +94 to +95
'aria-hidden': true,
focusable: false,
Copy link
Member

@sirreal sirreal Apr 10, 2020

Choose a reason for hiding this comment

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

The React declarations require this change, these props are typed as booleans. I checked in a code sandbox and the following confirms this should produce the same HTML.

This JSX:

<>
      <svg aria-hidden="true" focusable="false" />
      <svg aria-hidden={ true } focusable={ false } />
</>

Produces this HTML:

<svg aria-hidden="true" focusable="false"></svg>
<svg aria-hidden="true" focusable="false"></svg>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for checking!

"declarationDir": "build-types"
},
"include": [ "src/**/*" ],
"references": [ { "path": "../element" } ]
Copy link
Member

Choose a reason for hiding this comment

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

That's the only project reference we need 👍

"@babel/runtime": "^7.9.2",
"@wordpress/element": "file:../element",
"classnames": "^2.2.5"

@sirreal sirreal added the npm Packages Related to npm packages label Apr 10, 2020
@ockham
Copy link
Contributor Author

ockham commented Apr 10, 2020

Thanks Jon!

I forgot to add a Changelog entry, will do now.

@ockham ockham merged commit 319a8fc into master Apr 10, 2020
@ockham ockham deleted the add/primitives-types branch April 10, 2020 16:52
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Apr 10, 2020
@ockham
Copy link
Contributor Author

ockham commented Apr 10, 2020

I made one mistake, isPressed is optional. Fixing in #21487.

ockham added a commit that referenced this pull request Apr 10, 2020
Includes a fix to `@wordpress/primitives` to make `SVG`'s `isPressed` prop optional (oversight in #21482).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Packages Related to npm packages [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants