Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

Refactoring capybara theme #594

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

ymaheshwari1
Copy link
Contributor

Related Issues

Closes #

Short Description and Why It's Useful

  1. Lazy hydrating some components (footer, bottom navigation, modals, notifications) in both the layouts (default and minimal).

Taken reference from vuestorefront/vsf-default#57

Screenshots of Visual Changes before/after (If There Are Any)

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and Currently Important Rules Acceptance

Copy link
Contributor

@AishwaryShrivastav AishwaryShrivastav left a comment

Choose a reason for hiding this comment

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

Hi @ymaheshwari1 Please add change log and add proper issue in the PR
Implementation looks good.
Thanks for this improvement.

MCookieNotification: () => import('theme/components/molecules/m-cookie-notification'),
MOfflineBadge: () => import('theme/components/molecules/m-offline-badge'),
OBottomNavigation: () => import('theme/components/molecules/m-offline-badge'),
OModal: () => import('theme/components/organisms/o-modal'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are they lazy laded when they are not conditionally loaded?

Copy link
Contributor Author

@ymaheshwari1 ymaheshwari1 Apr 14, 2021

Choose a reason for hiding this comment

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

Loading them lazily as using lazy-hydrate to render the components.


export default {
components: {
OHeaderMinimal,
OFooter
OFooter: () => import('theme/components/organisms/o-footer'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants