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

[PB] Page Builder #1872

Merged
merged 306 commits into from
Oct 18, 2019
Merged

[PB] Page Builder #1872

merged 306 commits into from
Oct 18, 2019

Conversation

davemacaulay
Copy link
Contributor

@davemacaulay davemacaulay commented Oct 9, 2019

Description

Introduce Page Builder support to PWA Studio

Related Issue

https://jira.corp.magento.com/browse/PB-1

Verification Steps

  1. Check that each property for each content type appearance renders correctly on PWA Venia storefront in desktop and mobile.
  2. Compare Venia PWA rendering of content type with Luma to ensure the experience is the same or better for each property of each appearance in desktop and mobile.
  3. Check each supported browser and on mobile that is supported according to PWA documentation.
  4. Acceptable performance has been validated

cspruiell and others added 30 commits September 24, 2019 15:10
- Update tests for associated content types
WIP pagebuilder docs setup in menu, etc.
# Conflicts:
#	packages/venia-ui/lib/components/RichContent/PageBuilder/__tests__/parseStorageHtml.spec.js
[PB] Master format parser for PWA Studio
# Conflicts:
#	packages/venia-ui/lib/components/RichContent/PageBuilder/ContentTypes/Image/image.js
- Include cleanup in useEffect hook
…into PB-19

# Conflicts:
#	packages/venia-ui/lib/components/RichContent/PageBuilder/config.js
#	packages/venia-ui/package.json
#	yarn.lock
Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

(3 of 4) (150 of 202 files reviewed)

Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

(4 of 4) (202 / 202 files reviewed)

@awilcoxa
Copy link

Appears homepage content is broken on develop if this is merged, we should resolve any issue that breaks current app functionality prior to merging.

Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

@davemacaulay Thanks for making all of those changes. This looks really good now. 👍

I noticed two more minor things, but otherwise we're good to go.

@davemacaulay
Copy link
Contributor Author

@awilcoxa @jimbo

Appears homepage content is broken on develop if this is merged, we should resolve any issue that breaks current app functionality prior to merging.

We've added some logic to show the category list if the page is empty or it has default content.

e35b463#diff-68c0ddcfb13cfa542ddc32f37857f438R30-R39

jimbo
jimbo previously approved these changes Oct 18, 2019
Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

Approving this from my end. Nice work all around. 👍

@dpatil-magento I think we can move this into your court now.

@davemacaulay
Copy link
Contributor Author

davemacaulay commented Oct 18, 2019

Approving this from my end. Nice work all around. 👍

@dpatil-magento I think we can move this into your court now.

Thanks @jimbo!

@dhaecker will QA this from our perspective before merging , @dpatil-magento there's no need for you to QA this (unless you wish too).

[PB][Docs] PB-36: Creating custom Page Builder components
@davemacaulay davemacaulay requested a review from dhaecker October 18, 2019 20:30
Copy link
Collaborator

@dhaecker dhaecker left a comment

Choose a reason for hiding this comment

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

QA approved

@zetlen
Copy link
Contributor

zetlen commented Oct 18, 2019

HONK HONK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:peregrine pkg:pwa-buildpack pkg:upward-js Pertains to upward-js reference implementation of UPWARD. pkg:venia-concept pkg:venia-ui version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump.
Projects
None yet
Development

Successfully merging this pull request may close these issues.