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

Implemented: header minimal as a functional component(#592) #593

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

Conversation

ymaheshwari1
Copy link
Contributor

Related Issues

The header-minimal has no state management required so we define it as a functional component in place of it.

Closes #592

Short Description and Why It's Useful

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.

Changes looks good to me, @Fifciu can you check once.

<div class="o-header-minimal">
<ALogo />
</div>
</template>

<script>
import ALogo from 'theme/components/atoms/a-logo';
import Vue from 'vue'

Vue.component('ALogo', ALogo)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is o-header-minimal component used in more than 1 place? If no, it might be better to import and register ALogo component in the parent. Ofc, not register globally but in components section

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the future we should use minimal header in checkout as well as an eCommerce best practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, for now we can register the ALogo component in the Minimal layout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the future, we will handle the ALogo registration in all the components wherever the minimal header is used.
Also, if we are thinking of registering the SFUI library globally, then I think there will be no need to register the ALogo component even in the parent component.

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.

Using Header minimal as a functional component
4 participants