-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(#679): update menu when logged in on static #681
Conversation
🦋 Changeset detectedLatest commit: bec8514 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe changes introduce a new feature to the "druxt-menu" module, enabling the update of menu items for logged-in users on a statically generated site. This is achieved by adding a Changes
Poem
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .changeset/flat-birds-play.md (1 hunks)
- packages/menu/src/components/DruxtMenu.vue (1 hunks)
Additional comments: 2
.changeset/flat-birds-play.md (1)
- 1-5: The changeset file correctly specifies the module being patched and provides a brief description of the feature being added. Ensure that the feature number (Menu not showing authenticated links #679) corresponds to the correct issue or feature request in your tracking system.
packages/menu/src/components/DruxtMenu.vue (1)
- 222-228: This new
mounted
lifecycle hook checks if the user is logged in and if the page is statically generated. If both conditions are met, it clears thevalue
property and fetches the updated menu data using thedruxt.fetchData
method. This change modifies the behavior of the component when it is mounted under specific conditions, ensuring that the menu items are updated appropriately for logged-in users on static pages.However, it's important to note that this code assumes the existence of
$auth
and$store.app.context.isStatic
properties. If these properties are not guaranteed to exist in all contexts where this component is used, it would be safer to add additional checks to prevent potential errors.- if ((this.$auth || {}).loggedIn && this.$store.app.context.isStatic) { + if (this.$auth?.loggedIn && this.$store?.app?.context?.isStatic) {
Failing tests appear to be passing locally. Investigating. |
Tests are also failing on develop. Maintenance required. |
13d0f73
to
15f5c22
Compare
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .changeset/flat-birds-play.md (1 hunks)
- packages/menu/src/components/DruxtMenu.vue (1 hunks)
Additional comments: 1
.changeset/flat-birds-play.md (1)
- 1-5: This changeset file correctly specifies the module that is being patched and provides a brief description of the feature being added. Ensure that the feature number (Menu not showing authenticated links #679) corresponds to the correct issue or feature request in your tracking system.
}, | ||
}, | ||
|
||
mounted() { | ||
// If logged in and statically generated, re-fetch the menu. | ||
if ((this.$auth || {}).loggedIn && this.$store.app.context.isStatic) { | ||
this.value = undefined | ||
const settings = this.$options.druxt.settings(this, this.component.settings) | ||
this.$options.druxt.fetchData.call(this, settings) | ||
} | ||
}, |
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.
The mounted
lifecycle hook has been added to check if the user is logged in and if the page is statically generated. If both conditions are met, it clears the value
property and fetches the updated menu data using the druxt.fetchData
method. This ensures that the menu items are updated appropriately for logged-in users on static pages.
However, it's important to note that this approach assumes that the this.$auth
object and this.$store.app.context.isStatic
property are always available and correctly set, which might not always be the case. It would be safer to add some error handling or checks to ensure these properties exist and are in the expected format before using them.
mounted() {
// If logged in and statically generated, re-fetch the menu.
- if ((this.$auth || {}).loggedIn && this.$store.app.context.isStatic) {
+ if (this.$auth?.loggedIn && this.$store?.app?.context?.isStatic) {
this.value = undefined
const settings = this.$options.druxt.settings(this, this.component.settings)
this.$options.druxt.fetchData.call(this, settings)
}
},
Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
}, | |
}, | |
mounted() { | |
// If logged in and statically generated, re-fetch the menu. | |
if ((this.$auth || {}).loggedIn && this.$store.app.context.isStatic) { | |
this.value = undefined | |
const settings = this.$options.druxt.settings(this, this.component.settings) | |
this.$options.druxt.fetchData.call(this, settings) | |
} | |
}, | |
mounted() { | |
// If logged in and statically generated, re-fetch the menu. | |
if (this.$auth?.loggedIn && this.$store?.app?.context?.isStatic) { | |
this.value = undefined | |
const settings = this.$options.druxt.settings(this, this.component.settings) | |
this.$options.druxt.fetchData.call(this, settings) | |
} | |
}, |
Codecov Report
@@ Coverage Diff @@
## develop #681 +/- ##
========================================
Coverage 96.14% 96.14%
========================================
Files 91 91
Lines 2362 2362
Branches 509 509
========================================
Hits 2271 2271
Misses 76 76
Partials 15 15
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/menu/src/components/DruxtMenu.vue (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/menu/src/components/DruxtMenu.vue
Types of changes
Description
Checklist:
Screenshots/Media:
Summary by CodeRabbit