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

Editor: Move assets/src/edit-story to story-editor and wp-story-editor packages #8561

Merged
merged 13 commits into from
Aug 10, 2021

Conversation

mohdsayed
Copy link
Contributor

@mohdsayed mohdsayed commented Aug 3, 2021

Context

This PR only moves assets/src/editor-story to packages/story-editor/src directory and does not contain any code changes.

Summary

Relevant Technical Choices

To-do

User-facing changes

Testing Instructions

QA

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

UAT

  • UAT should use the same steps as above.

This PR can be tested by following these steps:

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This PR contains automated tests (unit, integration, and/or e2e) to verify the code works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Partially addresses #6050

@mohdsayed mohdsayed added the Type: Infrastructure Changes impacting testing infrastructure or build tooling label Aug 3, 2021
@google-cla google-cla bot added the cla: yes label Aug 3, 2021
@mohdsayed mohdsayed self-assigned this Aug 3, 2021
@mohdsayed mohdsayed changed the title Move to assets/src/editor-story to packages/story-editor/src Move assets/src/editor-story to packages/story-editor/src Aug 3, 2021
@spacedmonkey
Copy link
Contributor

spacedmonkey commented Aug 3, 2021

When i have moved files in the past,I have used PHPStorm. This find references to files in other places and will try and update the values for you. There will be hardcoded reference in the dashboard, webpack build files and other other places.

This PR in it's current state is unmergable as most of the CI is failing.

@swissspidy
Copy link
Collaborator

This PR in its current state is unmergable as most of the CI is failing.

I think this is expected, as it's still a draft.

@swissspidy swissspidy added the Package: Story Editor /packages/story-editor label Aug 3, 2021
@mohdsayed
Copy link
Contributor Author

@spacedmonkey Thank you, yes I think I am aware of some of those. This PR doesn't contain any code changes, it only moves the large chunk to packages so that it's easier to review the upcoming PRs with code changes which I will create against this branch. I would finally merge all those PRs in this branch and ensure that CI is passing before we merge it in main.

@mohdsayed mohdsayed changed the title Move assets/src/editor-story to packages/story-editor/src [WIP] Move assets/src/editor-story to packages/story-editor/src Aug 4, 2021
@mohdsayed mohdsayed changed the title [WIP] Move assets/src/editor-story to packages/story-editor/src Move assets/src/editor-story to packages/story-editor/src Aug 4, 2021
@swissspidy swissspidy changed the title Move assets/src/editor-story to packages/story-editor/src Move assets/src/edit-story to packages/story-editor/src Aug 4, 2021
@mohdsayed mohdsayed marked this pull request as ready for review August 6, 2021 04:45
Co-authored-by: Jonny Harris <jon@spacedmonkey.co.uk>
@google-cla
Copy link

google-cla bot commented Aug 9, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2021

Size Change: -1.16 MB (-33%) 🎉

Total Size: 2.33 MB

Filename Size Change
assets/css/edit-story-rtl.css 0 B -1.23 kB (removed) 🏆
assets/css/edit-story.css 0 B -1.23 kB (removed) 🏆
assets/js/edit-story.js 0 B -1.33 MB (removed) 🏆
assets/js/stories-dashboard.js 58.7 kB -1.16 MB (-95%) 🏆
assets/js/vendors-chunk-resize-observer-polyfill-edit-story-********************.js 0 B -2.55 kB (removed) 🏆
assets/js/vendors-edit-story-stories-dashboard-********************.js 0 B -230 kB (removed) 🏆
assets/css/vendors-stories-dashboard-wp-story-editor-rtl.css 706 B +706 B (new file) 🆕
assets/css/vendors-stories-dashboard-wp-story-editor.css 706 B +706 B (new file) 🆕
assets/css/wp-story-editor-rtl.css 608 B +608 B (new file) 🆕
assets/css/wp-story-editor.css 609 B +609 B (new file) 🆕
assets/js/stories-dashboard-wp-story-editor-********************.js 1.31 MB +1.31 MB (new file) 🆕
assets/js/vendors-stories-dashboard-wp-story-editor-********************.js 240 kB +240 kB (new file) 🆕
assets/js/wp-story-editor.js 4.19 kB +4.19 kB (new file) 🆕
ℹ️ View Unchanged
Filename Size Change
assets/css/carousel-view-rtl.css 701 B 0 B
assets/css/carousel-view.css 701 B 0 B
assets/css/stories-dashboard-rtl.css 658 B 0 B
assets/css/stories-dashboard.css 659 B 0 B
assets/css/web-stories-block-rtl.css 4.43 kB 0 B
assets/css/web-stories-block.css 4.47 kB 0 B
assets/css/web-stories-embed-rtl.css 288 B 0 B
assets/css/web-stories-embed.css 288 B 0 B
assets/css/web-stories-list-styles-rtl.css 2.3 kB 0 B
assets/css/web-stories-list-styles.css 2.32 kB 0 B
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B 0 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B 0 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B 0 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B 0 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B 0 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B 0 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B 0 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B 0 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B 0 B
assets/css/web-stories-theme-style-twentyten.css 143 B 0 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 325 B 0 B
assets/css/web-stories-theme-style-twentytwentyone.css 326 B 0 B
assets/css/web-stories-widget-rtl.css 484 B 0 B
assets/css/web-stories-widget.css 484 B 0 B
assets/js/carousel-view.js 3.72 kB 0 B
assets/js/chunk-colorthief-********************.js 2.61 kB 0 B
assets/js/chunk-focus-visible-********************.js 1 kB +1 B (0%)
assets/js/chunk-fonts-********************.js 46.3 kB 0 B
assets/js/chunk-web-stories-template-0-********************.js 475 B -1 B (0%)
assets/js/chunk-web-stories-template-10-********************.js 8.66 kB 0 B
assets/js/chunk-web-stories-template-100-********************.js 9.65 kB 0 B
assets/js/chunk-web-stories-template-102-********************.js 481 B 0 B
assets/js/chunk-web-stories-template-106-********************.js 8.27 kB 0 B
assets/js/chunk-web-stories-template-108-********************.js 459 B +1 B (0%)
assets/js/chunk-web-stories-template-112-********************.js 8.52 kB 0 B
assets/js/chunk-web-stories-template-114-********************.js 451 B 0 B
assets/js/chunk-web-stories-template-118-********************.js 6.96 kB 0 B
assets/js/chunk-web-stories-template-12-********************.js 506 B 0 B
assets/js/chunk-web-stories-template-120-********************.js 531 B +1 B (0%)
assets/js/chunk-web-stories-template-124-********************.js 6.75 kB 0 B
assets/js/chunk-web-stories-template-126-********************.js 450 B 0 B
assets/js/chunk-web-stories-template-130-********************.js 10.6 kB 0 B
assets/js/chunk-web-stories-template-132-********************.js 476 B 0 B
assets/js/chunk-web-stories-template-136-********************.js 6.76 kB 0 B
assets/js/chunk-web-stories-template-138-********************.js 534 B +1 B (0%)
assets/js/chunk-web-stories-template-142-********************.js 6.4 kB 0 B
assets/js/chunk-web-stories-template-144-********************.js 473 B +1 B (0%)
assets/js/chunk-web-stories-template-148-********************.js 7.28 kB 0 B
assets/js/chunk-web-stories-template-150-********************.js 459 B 0 B
assets/js/chunk-web-stories-template-154-********************.js 8.16 kB 0 B
assets/js/chunk-web-stories-template-156-********************.js 491 B +1 B (0%)
assets/js/chunk-web-stories-template-16-********************.js 8.77 kB 0 B
assets/js/chunk-web-stories-template-160-********************.js 8.14 kB 0 B
assets/js/chunk-web-stories-template-162-********************.js 506 B 0 B
assets/js/chunk-web-stories-template-166-********************.js 7.53 kB 0 B
assets/js/chunk-web-stories-template-168-********************.js 444 B 0 B
assets/js/chunk-web-stories-template-172-********************.js 9.19 kB 0 B
assets/js/chunk-web-stories-template-174-********************.js 480 B 0 B
assets/js/chunk-web-stories-template-178-********************.js 12 kB 0 B
assets/js/chunk-web-stories-template-18-********************.js 473 B 0 B
assets/js/chunk-web-stories-template-180-********************.js 431 B +1 B (0%)
assets/js/chunk-web-stories-template-184-********************.js 8.64 kB 0 B
assets/js/chunk-web-stories-template-186-********************.js 505 B 0 B
assets/js/chunk-web-stories-template-190-********************.js 7.29 kB 0 B
assets/js/chunk-web-stories-template-192-********************.js 497 B 0 B
assets/js/chunk-web-stories-template-196-********************.js 8.59 kB 0 B
assets/js/chunk-web-stories-template-198-********************.js 513 B 0 B
assets/js/chunk-web-stories-template-202-********************.js 11.1 kB 0 B
assets/js/chunk-web-stories-template-204-********************.js 462 B 0 B
assets/js/chunk-web-stories-template-208-********************.js 5.98 kB 0 B
assets/js/chunk-web-stories-template-210-********************.js 517 B 0 B
assets/js/chunk-web-stories-template-214-********************.js 7.15 kB 0 B
assets/js/chunk-web-stories-template-216-********************.js 489 B 0 B
assets/js/chunk-web-stories-template-22-********************.js 7.86 kB 0 B
assets/js/chunk-web-stories-template-220-********************.js 8.77 kB 0 B
assets/js/chunk-web-stories-template-222-********************.js 491 B 0 B
assets/js/chunk-web-stories-template-226-********************.js 7.47 kB 0 B
assets/js/chunk-web-stories-template-228-********************.js 454 B -1 B (0%)
assets/js/chunk-web-stories-template-232-********************.js 6.64 kB 0 B
assets/js/chunk-web-stories-template-234-********************.js 483 B +1 B (0%)
assets/js/chunk-web-stories-template-238-********************.js 8.02 kB 0 B
assets/js/chunk-web-stories-template-24-********************.js 492 B +1 B (0%)
assets/js/chunk-web-stories-template-240-********************.js 503 B -1 B (0%)
assets/js/chunk-web-stories-template-244-********************.js 9.88 kB 0 B
assets/js/chunk-web-stories-template-246-********************.js 468 B -1 B (0%)
assets/js/chunk-web-stories-template-250-********************.js 4.64 kB 0 B
assets/js/chunk-web-stories-template-252-********************.js 511 B -1 B (0%)
assets/js/chunk-web-stories-template-256-********************.js 8.02 kB 0 B
assets/js/chunk-web-stories-template-258-********************.js 490 B +1 B (0%)
assets/js/chunk-web-stories-template-262-********************.js 7.81 kB 0 B
assets/js/chunk-web-stories-template-264-********************.js 430 B 0 B
assets/js/chunk-web-stories-template-268-********************.js 8.15 kB 0 B
assets/js/chunk-web-stories-template-270-********************.js 526 B -1 B (0%)
assets/js/chunk-web-stories-template-274-********************.js 9.74 kB 0 B
assets/js/chunk-web-stories-template-276-********************.js 471 B 0 B
assets/js/chunk-web-stories-template-28-********************.js 9.01 kB 0 B
assets/js/chunk-web-stories-template-280-********************.js 9.23 kB 0 B
assets/js/chunk-web-stories-template-282-********************.js 465 B 0 B
assets/js/chunk-web-stories-template-286-********************.js 13.8 kB 0 B
assets/js/chunk-web-stories-template-288-********************.js 484 B 0 B
assets/js/chunk-web-stories-template-292-********************.js 5.35 kB 0 B
assets/js/chunk-web-stories-template-294-********************.js 509 B -2 B (0%)
assets/js/chunk-web-stories-template-298-********************.js 8.32 kB 0 B
assets/js/chunk-web-stories-template-30-********************.js 493 B 0 B
assets/js/chunk-web-stories-template-300-********************.js 445 B -1 B (0%)
assets/js/chunk-web-stories-template-304-********************.js 8.56 kB +1 B (0%)
assets/js/chunk-web-stories-template-34-********************.js 6.76 kB 0 B
assets/js/chunk-web-stories-template-36-********************.js 500 B 0 B
assets/js/chunk-web-stories-template-4-********************.js 10.4 kB 0 B
assets/js/chunk-web-stories-template-40-********************.js 7.99 kB 0 B
assets/js/chunk-web-stories-template-42-********************.js 515 B 0 B
assets/js/chunk-web-stories-template-46-********************.js 8.1 kB 0 B
assets/js/chunk-web-stories-template-48-********************.js 465 B 0 B
assets/js/chunk-web-stories-template-52-********************.js 6.65 kB 0 B
assets/js/chunk-web-stories-template-54-********************.js 472 B +1 B (0%)
assets/js/chunk-web-stories-template-58-********************.js 8.06 kB 0 B
assets/js/chunk-web-stories-template-6-********************.js 476 B 0 B
assets/js/chunk-web-stories-template-60-********************.js 455 B +1 B (0%)
assets/js/chunk-web-stories-template-64-********************.js 7.06 kB 0 B
assets/js/chunk-web-stories-template-66-********************.js 506 B 0 B
assets/js/chunk-web-stories-template-70-********************.js 7.15 kB 0 B
assets/js/chunk-web-stories-template-72-********************.js 467 B +1 B (0%)
assets/js/chunk-web-stories-template-76-********************.js 7.75 kB 0 B
assets/js/chunk-web-stories-template-78-********************.js 514 B +1 B (0%)
assets/js/chunk-web-stories-template-82-********************.js 9.96 kB 0 B
assets/js/chunk-web-stories-template-84-********************.js 479 B 0 B
assets/js/chunk-web-stories-template-88-********************.js 8.01 kB 0 B
assets/js/chunk-web-stories-template-90-********************.js 517 B 0 B
assets/js/chunk-web-stories-template-94-********************.js 8.88 kB 0 B
assets/js/chunk-web-stories-template-96-********************.js 425 B 0 B
assets/js/chunk-web-stories-textset-0-********************.js 5.29 kB 0 B
assets/js/chunk-web-stories-textset-1-********************.js 6.79 kB 0 B
assets/js/chunk-web-stories-textset-2-********************.js 7.91 kB 0 B
assets/js/chunk-web-stories-textset-3-********************.js 15.4 kB -1 B (0%)
assets/js/chunk-web-stories-textset-4-********************.js 4.4 kB 0 B
assets/js/chunk-web-stories-textset-5-********************.js 5.71 kB +1 B (0%)
assets/js/chunk-web-stories-textset-6-********************.js 5.52 kB 0 B
assets/js/chunk-web-stories-textset-7-********************.js 10.4 kB 0 B
assets/js/imgareaselect.js 4.14 kB 0 B
assets/js/lightbox.js 991 B 0 B
assets/js/tinymce-button.js 3.49 kB 0 B
assets/js/vendors-chunk-ffmpeg-********************.js 5.6 kB -1 B (0%)
assets/js/vendors-chunk-html-to-image-********************.js 4.64 kB 0 B
assets/js/vendors-chunk-react-calendar-********************.js 11.7 kB 0 B
assets/js/vendors-chunk-react-color-********************.js 42.9 kB +2 B (0%)
assets/js/vendors-chunk-web-animations-js-********************.js 14.6 kB 0 B
assets/js/web-stories-activation-notice.js 23.3 kB 0 B
assets/js/web-stories-block.js 18.2 kB 0 B
assets/js/web-stories-embed.js 493 B 0 B
assets/js/web-stories-widget.js 984 B 0 B

compressed-size-action

@swissspidy
Copy link
Collaborator

@spacedmonkey can you please reply to the CLA bot?

@GoogleForCreators GoogleForCreators deleted a comment from google-cla bot Aug 9, 2021
@GoogleForCreators GoogleForCreators deleted a comment from google-cla bot Aug 9, 2021
@GoogleForCreators GoogleForCreators deleted a comment from google-cla bot Aug 9, 2021
@GoogleForCreators GoogleForCreators deleted a comment from google-cla bot Aug 9, 2021
@GoogleForCreators GoogleForCreators deleted a comment from google-cla bot Aug 9, 2021
@GoogleForCreators GoogleForCreators deleted a comment from google-cla bot Aug 9, 2021
@GoogleForCreators GoogleForCreators deleted a comment from google-cla bot Aug 9, 2021
webpack.config.cjs Outdated Show resolved Hide resolved
Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

import Modal from 'react-modal';

The above is references, 3 times in the dashboard. But Modal is exported from the editor package. This should be changed to read from edit package and removed the main react-modal package.json

@swissspidy
Copy link
Collaborator

import Modal from 'react-modal';

The above is references, 3 times in the dashboard. But Modal is exported from the editor package. This should be changed to read from edit package and removed the main react-modal package.json

Did you mean import from design system?
We are working on removing any inter-dependencies between dashboard and editor

@spacedmonkey
Copy link
Contributor

Did you mean import from design system?

I asked the same question. #8561

ATM, design-system, editor and dashboard all import the package. Seems a messy. Makes sense to important in just one place. Design-system makes sense to me.

Sayed Taqui and others added 3 commits August 10, 2021 10:11
…or` in webpack and other places (#8636)

* Change script handle from edit-story to wp-story-editor

* Update webpack handle in karma
@mohdsayed
Copy link
Contributor Author

mohdsayed commented Aug 10, 2021

JFI, "PHP Unit Tests / PHP 7.4 - WP trunk (pull_request)" is failing after b9aff55 which is the only PHP change. But I think its not because of this change, it's failing on other PRs having PHP changes too.

* Remove localStore from edit-story and move to design system

* Update all localStore local references to design-system module

* Update more local reference of localStore

* Mock localStore from design-system package in jest
@mohdsayed mohdsayed requested a review from spacedmonkey August 10, 2021 09:29
@swissspidy
Copy link
Collaborator

swissspidy commented Aug 10, 2021

@sayedtaqui It's a known issue, see https://core.trac.wordpress.org/ticket/46149. Tests against trunk are marked as "experimental" in CI (using continue-on-error) for reasons like that.

@codecov
Copy link

codecov bot commented Aug 10, 2021

Codecov Report

Merging #8561 (c065940) into main (5f98c58) will increase coverage by 17.26%.
The diff coverage is 25.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #8561       +/-   ##
===========================================
+ Coverage   61.83%   79.10%   +17.26%     
===========================================
  Files         881     1336      +455     
  Lines       15193    18959     +3766     
===========================================
+ Hits         9395    14998     +5603     
+ Misses       5798     3961     -1837     
Flag Coverage Δ
karmatests 77.41% <33.33%> (?)
unittests 72.29% <0.00%> (+10.45%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
assets/src/dashboard/app/api/useStoryApi.js 75.00% <0.00%> (ø)
assets/src/dashboard/app/api/useTemplateApi.js 29.31% <ø> (ø)
assets/src/dashboard/app/font/fontProvider.js 79.16% <ø> (+79.16%) ⬆️
...hboard/app/views/exploreTemplates/content/index.js 85.71% <ø> (ø)
...ashboard/app/views/savedTemplates/content/index.js 66.66% <ø> (ø)
...rc/dashboard/app/views/shared/useTelemetryOptIn.js 93.75% <ø> (+18.75%) ⬆️
...s/src/dashboard/app/views/templateDetails/index.js 82.92% <ø> (+82.92%) ⬆️
...sets/src/dashboard/components/cardGallery/index.js 97.72% <ø> (ø)
...c/dashboard/components/cardGridItem/cardPreview.js 95.12% <ø> (+7.31%) ⬆️
assets/src/dashboard/index.js 0.00% <0.00%> (ø)
... and 1948 more

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

GREAT WORK!

@swissspidy swissspidy changed the title Move assets/src/edit-story to packages/story-editor/src Editor: Move assets/src/edit-story to story-editor and wp-story-editor packages Aug 10, 2021
@swissspidy swissspidy merged commit c3a3b99 into main Aug 10, 2021
@swissspidy swissspidy deleted the feature/package-story-editor branch August 10, 2021 10:51
@mohdsayed mohdsayed mentioned this pull request Sep 6, 2021
17 tasks
@swissspidy swissspidy mentioned this pull request Sep 9, 2021
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: Story Editor /packages/story-editor Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants