Skip to content
New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

enhancement(load): Branded first renders #1275

Merged
merged 35 commits into from
Jun 24, 2019
Merged

Conversation

zetlen
Copy link
Contributor

@zetlen zetlen commented May 27, 2019

Description

Refactor Mustache templates to share a small critical CSS block and use it to render a branded first paint.

Add a general concept of resource.name to UPWARD, enabling the loading indicator to be a shared partial which uses resource.name and can therefore be customized per page.

Refactor noscript and 404 pages to use the same basic system.

Related Issue

Closes #1272

Verification Steps

  1. Go to a nonexistent page with an HTML extension, like /nothing.html. Verify Not Found page.
    magento-venia-concept-fquwg local pwadev_8436_nothing html(iPhone 5_SE)
    magento-venia-concept-fquwg local pwadev_8436_nothing html(iPhone 5_SE) (1)

  2. Edit packages/venia-concept/templates/seed-bundles.mst and type garbage in the src attribute so the app doesn't load.

{{#assets.bundles.load}}
    <script defer src="bsdasbd{{.}}"></script>
{{/assets.bundles.load}}
  1. Go to homepage. Verify branded first render.

magento-venia-concept-fquwg local pwadev_8436_(iPhone 5_SE)
magento-venia-concept-fquwg local pwadev_8436_(iPhone 5_SE) (1)

  1. Go to category page: /venia-tops.html. Verify branded first render.
    magento-venia-concept-fquwg local pwadev_8436_venia-tops html(iPhone 5_SE)

  2. Go to product page: /carmina-earrings.html. Verify branded first render.
    magento-venia-concept-fquwg local pwadev_8436_carmina-earrings html(iPhone 5_SE)

  3. Open Chrome Developer tools, open the Command Palette (Cmd-Shift-P on OSX) and type "Disable JavaScript". Press Enter.

  4. Refresh page. Verify branded no-script experience.
    magento-venia-concept-fquwg local pwadev_8436_carmina-earrings html(iPhone 5_SE) (1)
    magento-venia-concept-fquwg local pwadev_8436_carmina-earrings html(iPhone 5_SE) (2)

How Have YOU Tested this?

Portrait and landscape orientation on many, many browsers and devices.

Proposed Labels for Change Type/Package

  • major (e.g x.0.0 - a breaking change)
  • minor (e.g 0.x.0 - a backwards compatible addition)
  • patch (e.g 0.0.x - a bug fix)

Checklist:

  • I have updated the documentation accordingly, if necessary.
  • I have added tests to cover my changes, if necessary.

@zetlen zetlen requested a review from jimbo May 27, 2019 03:33
@vercel
Copy link

vercel bot commented May 27, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://venia-git-zetlen-branded-first-render.magento-research1.now.sh

@zetlen zetlen requested review from supernova-at and sirugh May 27, 2019 03:33
@zetlen zetlen changed the base branch from develop to non-blocking-css-font May 27, 2019 15:18
@zetlen zetlen force-pushed the zetlen/branded-first-render branch 2 times, most recently from cafb98f to f82a002 Compare May 27, 2019 15:23
@vercel vercel bot temporarily deployed to staging May 27, 2019 15:23 Inactive
@zetlen zetlen force-pushed the zetlen/branded-first-render branch from f82a002 to bc07407 Compare May 27, 2019 15:24
@coveralls
Copy link

coveralls commented May 27, 2019

Coverage Status

Coverage remained the same at 78.661% when pulling 5802479 on zetlen/branded-first-render into 7655bf3 on non-blocking-css-font.

@soumya-ashok
Copy link

soumya-ashok commented May 28, 2019

@zetlen This is the mockup I have for a general "Page not found" use-case. Please let me know if any modifications need to be made for this specific instance.

Page not found

@soumya-ashok
Copy link

@zetlen On the loading indicator and text, there is a size and treatment for both that we are following on other pages, so would be good to keep that consistent here as well.

@zetlen zetlen force-pushed the zetlen/branded-first-render branch from 85afbb9 to 6aed1c9 Compare May 29, 2019 17:34
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Jun 18, 2019

Messages
📖 We are currently working on automating the PR metadata checks. Until that time, you may see failures related to labels/description/linked issues/etc even if you have fixed the problem. Failures will persist until the next push (assuming they are fixed).

Generated by 🚫 dangerJS against ffff99d

@zetlen
Copy link
Contributor Author

zetlen commented Jun 18, 2019

By page name I mean - "Home" "Product details" "Search results" etc. - will this work?

Text color #212121

branded-render.png

@soumya-ashok The page name can include the name of the actual entity: whether it's the product ("Carmina Earrings") or the category "Accessories" or a search result for the query "green". Do you want to not include that dynamic data and just say "Product details", "Category" above the loading spinner?

@soumya-ashok
Copy link

Yes, my preference would be to keep it generic for this first iteration, since we are working on a more detailed strategy for the future.

@zetlen
Copy link
Contributor Author

zetlen commented Jun 18, 2019

@soumya-ashok OK, update pushed! In both the first-paint and intra-page loading indicators, you should now have a fully vertically centered UI.

@sirugh
Copy link
Contributor

sirugh commented Jun 21, 2019

@dpatil-magento merge conflict and any missing stuff has been added!

@zetlen zetlen added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Jun 24, 2019
@vercel vercel bot temporarily deployed to staging June 24, 2019 20:47 Inactive
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.

@zetlen Looks good to me now; thanks for making changes. 👍

@zetlen zetlen merged commit 44861ad into develop Jun 24, 2019
@zetlen zetlen deleted the zetlen/branded-first-render branch June 24, 2019 20:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
pkg:venia-concept version: Minor This changeset includes functionality added in a backwards compatible manner.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: Unloaded states have inconsistent style
8 participants