-
Notifications
You must be signed in to change notification settings - Fork 83
Improved: code to display the filter count on initial load (#554) #560
base: develop
Are you sure you want to change the base?
Improved: code to display the filter count on initial load (#554) #560
Conversation
pages/Category.vue
Outdated
@@ -223,6 +223,8 @@ import { | |||
|
|||
const THEME_PAGE_SIZE = 12; | |||
const LAZY_LOADING_ACTIVATION_BREAKPOINT = 1024; | |||
var unsubscribeFromStoreAction = '' | |||
var aggs = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using global variables is not a good idea. It might cause race conditions in concurrent requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fifciu
The other solution can be to call the composeInitialPageState in the beforeRouteEnter hook. You can see the solution in this commit: 57cb6cd
Also, another thing that I can think of is to maintain a state for storing the aggregations and then using that state in our component, and thus we also not need to subscribe to the action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fifciu I have updated the solution to make use of state and getter to use the aggregations, please review.
vuestorefront/vue-storefront#5392
e93a4f6
to
95fc967
Compare
this.aggregations[aggregation].buckets.find( | ||
this.getAggregations && | ||
this.getAggregations[aggregation] && | ||
this.getAggregations[aggregation].buckets.find( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add Array.isArray(this.getAggregations[aggregation].buckets)
check too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the check for the aggregations array
@Fifciu can you look into this also, changes looks good to me |
@@ -455,17 +454,11 @@ export default { | |||
} | |||
}, | |||
mounted () { | |||
this.unsubscribeFromStoreAction = this.$store.subscribeAction(action => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about this part? Have you test it manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I have tested it. Actually, on the initial load of the category page, only two specific actions are getting subscribed so, I think this part is of no use.
Related Issues
#554
Closes #554
Short Description and Why It's Useful
The changes will help in displaying the filter count on the initial load of category page.
Screenshots of Visual Changes before/after (If There Are Any)
IMPORTANT NOTICE - Remember to update
CHANGELOG.md
with description of your changeContribution and Currently Important Rules Acceptance