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

refactor: Remove legacy social plugin, PDP, product grid and WYSIWYG code #5394

Merged

Conversation

loan-laux
Copy link
Collaborator

@loan-laux loan-laux commented Jul 31, 2019

Resolves #5393
Impact: major
Type: performance|refactor

Issue

First of all, the Operator UI still has a Social section, which used to serve the purpose of adding social "share" buttons on the PDP, back in the 1.x days. This is not used anymore and this feature being present in Catalyst may confuse newcomers.

Second, lots of unused code relative to the legacy PDP, the product grid and the old WYSIWYG admin is still in this repo.

Solution

We need to remove the unused parts, which will help reduce the client bundle size, improving both Catalyst and customer login UI loading times by a little bit. Not to mention a small gain in DX as well.

Breaking changes

  • The social plugin is no more.
  • The following legacy components were removed:
    • productDetail
    • ProductField
    • ProductTags
    • ProductMetadata
    • PriceRange
    • VariantList
    • SocialContainer
    • VariantListContainer
    • ProductNotFound
  • In the default-theme plugin, all the CSS files under the products directory were removed, with the exception of products/variantForm.less which still has literally 3 rules that are still being used.

Testing

  1. Open the Operator UI
  2. Notice that everything works just as it used to
  3. Notice that the client bundle is a bit lighter

Before:
Screen Shot 2019-07-31 at 15 32 26

After:
Screen Shot 2019-07-31 at 14 48 52

@spencern spencern requested a review from kieckhafer July 31, 2019 14:34
@kieckhafer
Copy link
Member

kieckhafer commented Jul 31, 2019

Thanks @loan-laux! Can you please take care of the DCO issues before I dive into this: https://github.com/reactioncommerce/reaction/runs/181723922

Thanks!

@machikoyasuda
Copy link
Contributor

Actions to take after this PR is merged:

  • Update the docs to remove any instructions for the Social plugin
  • Update the docs to remove any old PDP/WYSIWIG component docs

Copy link
Member

@kieckhafer kieckhafer 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 overall! Thanks for all of this @loan-laux!

One comment here about one last LESS issue, and then the DCO fix, and this should be good to merge.

@loan-laux
Copy link
Collaborator Author

@kieckhafer Removed the variantForm.less file and fixed DCO.

Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
@kieckhafer kieckhafer merged commit b571b93 into reactioncommerce:develop Aug 2, 2019
@kieckhafer kieckhafer mentioned this pull request Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants