/gutenberg

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Framework: Using the wordpress a11y package instead of the global #2425

Merged
merged 1 commit into from Aug 25, 2017
+5 −3

Conversation

Reviewers

@aduth aduth

Assignees

@youknowriad youknowriad

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@youknowriad @aduth
@youknowriad
Collaborator

youknowriad commented Aug 15, 2017

No description provided.

@youknowriad youknowriad self-assigned this Aug 15, 2017

@youknowriad youknowriad requested a review from aduth Aug 15, 2017

@aduth

aduth requested changes Aug 15, 2017
View changes

Should we remove the dependency here?

gutenberg/lib/client-assets.php

Lines 129 to 134 in acde984

wp_register_script(
'wp-components',
gutenberg_url( 'components/build/index.js' ),
array( 'wp-element', 'wp-a11y', 'wp-i18n', 'wp-utils' ),
filemtime( gutenberg_dir_path() . 'components/build/index.js' )
);

Additionally / alternatively, we might consider how we plan to reconcile the wp-a11y script included in core currently with the now-packaged variant, and whether the two can be merged. Might be a topic for a JavaScript Office Hours discussion (cc @omarreiss)

@codecov

codecov bot commented Aug 15, 2017 • edited

Codecov Report

Merging #2425 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2425   +/-   ##
======================================
  Coverage    30.1%   30.1%           
======================================
  Files         174     174           
  Lines        5288    5288           
  Branches      907     907           
======================================
  Hits         1592    1592           
  Misses       3132    3132           
  Partials      564     564
Impacted Files Coverage Δ
...ponents/higher-order/with-spoken-messages/index.js 66.66% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f6adb6...13b992b. Read the comment docs.
@youknowriad
Collaborator

youknowriad commented Aug 15, 2017

@aduth Good catch and right, maybe this is the first package to be used from the packages repo in Core.
@youknowriad
Collaborator

youknowriad commented Aug 25, 2017

@aduth Do you think we should merge this since the core integration won't be resolved today?
@aduth

aduth approved these changes Aug 25, 2017
View changes

Yes, especially since we're not clobbering the global at the moment anyways, just pulling direct from NPM.

@youknowriad youknowriad Framework: Using the wordpress a11y package instead of the global

All checks have passed

3 successful checks
@codecov
codecov/project — 30.1% remains the same compared to 5f6adb6
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
13b992b

@youknowriad youknowriad merged commit 4f5f96f into master Aug 25, 2017
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@mtias mtias deleted the use/wordpress-a11y-package branch Aug 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment