change linkClasses in FwbNavbarLink to be computed rather than only evaluated on mount to resolve #373 #375
Conversation
…nly evaluated on mount
WalkthroughThe change updates the Changes
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/FwbNavbar/FwbNavbarLink.vue (1)
40-45: Good change making linkClasses reactive with computed!Converting
linkClassesfrom a static value to a computed property is a great fix for issue #373. This ensures the class list updates reactively when theisActiveprop changes, rather than only being evaluated once on component mount.However, there are some code style issues to fix:
const linkClasses = computed(() => { - return twMerge( - defaultClasses, - props.isActive ? currentPageClasses: defaultStateClasses, - ) + return twMerge( + defaultClasses, + props.isActive ? currentPageClasses : defaultStateClasses, + ) })🧰 Tools
🪛 ESLint
[error] 41-41: Expected indentation of 2 spaces but found 4.
(@stylistic/indent)
[error] 42-42: Expected indentation of 4 spaces but found 8.
(@stylistic/indent)
[error] 43-43: Unexpected tab character.
(@stylistic/no-tabs)
[error] 43-43: Expected indentation of 4 spaces but found 1 tab.
(@stylistic/indent)
[error] 43-43: Operator ':' must be spaced.
(@stylistic/space-infix-ops)
[error] 44-44: Expected indentation of 2 spaces but found 4.
(@stylistic/indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/FwbNavbar/FwbNavbarLink.vue(1 hunks)
🧰 Additional context used
🪛 ESLint
src/components/FwbNavbar/FwbNavbarLink.vue
[error] 41-41: Expected indentation of 2 spaces but found 4.
(@stylistic/indent)
[error] 42-42: Expected indentation of 4 spaces but found 8.
(@stylistic/indent)
[error] 43-43: Unexpected tab character.
(@stylistic/no-tabs)
[error] 43-43: Expected indentation of 4 spaces but found 1 tab.
(@stylistic/indent)
[error] 43-43: Operator ':' must be spaced.
(@stylistic/space-infix-ops)
[error] 44-44: Expected indentation of 2 spaces but found 4.
(@stylistic/indent)
+ return from arrow function simplified
The linkClasses const declared in FwbNavbarLink is just an ordinary constant. It should be reactive as it relies on the value of a prop.
Fix changes it to a computed value such that it will change when the prop changes. I assume that would be expected behaviour.
Fixes #373
Summary by CodeRabbit
Summary by CodeRabbit