Add social media links on a left menu #247
Conversation
WalkthroughThe pull request introduces several modifications across multiple Vue components in the frontendmu-nuxt package. Key changes include formatting improvements, structural enhancements, and the addition of a new fixed navigation menu component. Specific components have been updated for better readability and accessibility, while some unused imports have been removed. Additionally, a new Twitter icon component has been added. Overall, these changes aim to enhance code clarity and maintainability without altering the public interface of the components. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Deploying frontend-mu-nuxt with
|
| Latest commit: |
863f51b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://20fec4ff.frontend-mu-staging.pages.dev |
| Branch Preview URL: | https://move-social-media-links-to-s.frontend-mu-staging.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (1)
packages/frontendmu-nuxt/layouts/default.vue (1)
2-2: Minor formatting change: Use of double quotesThe change from single quotes to double quotes for the string argument in
defineOgImageComponentis a minor formatting adjustment. This change doesn't affect functionality but may improve consistency with the project's coding style.Ensure this change aligns with the project's style guide for string quotation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- packages/frontendmu-nuxt/components/auth/LoggedUser.vue (4 hunks)
- packages/frontendmu-nuxt/components/icon/discord.vue (1 hunks)
- packages/frontendmu-nuxt/components/site/LeftMenu.vue (1 hunks)
- packages/frontendmu-nuxt/components/site/Navigation.vue (1 hunks)
- packages/frontendmu-nuxt/layouts/default.vue (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/frontendmu-nuxt/components/icon/discord.vue
- packages/frontendmu-nuxt/components/site/Navigation.vue
🧰 Additional context used
🔇 Additional comments (8)
packages/frontendmu-nuxt/layouts/default.vue (1)
9-9: New component added: SiteLeftMenuThe addition of the
SiteLeftMenucomponent aligns with the PR objective of incorporating social media links into a left menu. Its placement betweenSiteNavigationand the main content (slot) is logical for a left-side menu layout.To ensure the new component is properly implemented and contains the expected social media links, please run the following verification script:
This script will help verify:
- The existence of the SiteLeftMenu component file.
- The content of the SiteLeftMenu component.
- The presence of social media related code within the component.
Please review the script output to ensure the component is implemented as expected and includes the social media links as per the PR objectives.
packages/frontendmu-nuxt/components/auth/LoggedUser.vue (7)
2-15: LGTM: Improved code consistency and readabilityThe changes in the script setup section enhance code consistency by using double quotes for imports and adding a semicolon after the
checkIfLoggedInfunction call. The simplification of theuseAuthdestructuring is also a good cleanup, removing potentially unused variables.
21-24: LGTM: Improved BaseButton formattingThe reformatting of the BaseButton components enhances code readability. The vertical alignment of attributes makes it easier to scan and understand the component properties. The separation of desktop and mobile versions is maintained, ensuring consistent functionality across different screen sizes.
Also applies to: 30-34
43-50: Excellent: Improved formatting and accessibilityThe reformatting of the MenuButton's class attribute enhances code readability. More importantly, the addition of the
altattribute to the user avatar image is a significant improvement in accessibility. This change ensures that screen readers can provide context for the image, adhering to web accessibility best practices.
57-60: LGTM: Enhanced transition component readabilityThe splitting of the transition component's class attributes into multiple lines significantly improves code clarity. This change makes each transition class more visible and easier to understand, without altering the component's functionality.
70-75: LGTM: Improved class binding readabilityThe adjustment of class bindings using array syntax for the "My Profile" MenuItem enhances code readability. This approach is a Vue best practice for handling conditional classes, making the code more maintainable and easier to understand.
82-87: LGTM: Consistent improvement in class binding readabilityThe class binding adjustments for the "Logout" MenuItem mirror those made for the "My Profile" link. This consistency in approach across similar components is commendable, as it enhances overall code readability and maintainability.
Line range hint
1-112: Overall: Improved code quality with minor suggestion for accessibilityThe changes in this file significantly enhance code readability, consistency, and partially improve accessibility. The reformatting of components, consistent use of quotation marks, and improved class bindings all contribute to a more maintainable codebase.
The addition of an
altattribute to one of the avatar images is a step in the right direction for accessibility. To fully realize this improvement, ensure that all avatar images have appropriatealtattributes.These changes align well with the PR objectives, preparing the component for the integration of social media links in the left menu by improving its overall structure and readability.
| <template> | ||
| <nav | ||
| class="fixed bottom-[50px] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex" | ||
| > | ||
| <a | ||
| href="https://discord.gg/WxXW9Jvv6k?ref=frontend.mu" | ||
| class="text-[#5865f2] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconDiscord class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://github.com/Front-End-Coders-Mauritius?ref=frontend.mu" | ||
| class="text-[#333] text-sm md:text-normal dark:text-white hover:animate-bounce" | ||
| > | ||
| <IconGithub class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://chat.whatsapp.com/invite/0kQ2QX0ZQ0j1YQ4X6Q4Q4Q" | ||
| class="text-[#25D366] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconWhatsappCommunity class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://twitter.com/frontendmu" | ||
| class="text-[#1DA1F2] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconTwitter class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://www.instagram.com/frontend.mu/?ref=frontend.mu" | ||
| class="text-[#d6249f] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconInstagram class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://www.linkedin.com/company/81846464/admin/?ref=frontend.mu" | ||
| class="text-[#007db1] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconLinkedin class="w-6" /> | ||
| </a> | ||
| </nav> | ||
| </template> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance maintainability and reusability of the social media navigation
To improve the maintainability and reusability of your social media navigation component, consider the following suggestions:
-
Extract the social media links data into a separate configuration file or computed property. This will make it easier to update or add new links in the future.
-
Create a reusable component for social media links to reduce code duplication.
-
Explicitly import the icon components at the top of the file.
Here's an example of how you could implement these changes:
<template>
<nav
class="fixed bottom-[3rem] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex max-h-[80vh] overflow-y-auto"
aria-label="Social media links"
role="navigation"
>
<SocialLink
v-for="link in socialLinks"
:key="link.name"
v-bind="link"
/>
</nav>
</template>
<script>
import { defineComponent } from 'vue'
import SocialLink from './SocialLink.vue'
import IconDiscord from './icons/IconDiscord.vue'
import IconGithub from './icons/IconGithub.vue'
import IconWhatsappCommunity from './icons/IconWhatsappCommunity.vue'
import IconTwitter from './icons/IconTwitter.vue'
import IconInstagram from './icons/IconInstagram.vue'
import IconLinkedin from './icons/IconLinkedin.vue'
export default defineComponent({
name: 'LeftMenu',
components: {
SocialLink,
},
setup() {
const socialLinks = [
{
name: 'Discord',
href: 'https://discord.gg/WxXW9Jvv6k?ref=frontend.mu',
icon: IconDiscord,
color: 'text-[#5865f2]',
},
// Add other social media links here
]
return {
socialLinks,
}
},
})
</script>Then, create a SocialLink.vue component:
<template>
<a
:href="href"
:class="[color, 'text-sm md:text-normal hover:animate-bounce opacity-80 hover:opacity-100']"
rel="noopener noreferrer"
target="_blank"
tabindex="0"
:aria-label="`Join our ${name} community`"
@keydown.enter="$event.target.click()"
>
<component :is="icon" class="w-6" />
<span class="sr-only">{{ name }}</span>
</a>
</template>
<script>
import { defineComponent } from 'vue'
export default defineComponent({
name: 'SocialLink',
props: {
name: {
type: String,
required: true,
},
href: {
type: String,
required: true,
},
icon: {
type: Object,
required: true,
},
color: {
type: String,
required: true,
},
},
})
</script>These changes will significantly improve the maintainability and reusability of your social media navigation component.
🛠️ Refactor suggestion
Improve accessibility and SEO of the social media navigation
To enhance the accessibility and SEO of your social media navigation, consider the following suggestions:
-
Add visible text alongside icons for better SEO. You can use Tailwind's
sr-onlyclass to hide the text visually while keeping it available for screen readers and search engines. -
Implement keyboard navigation support by adding
tabindex="0"to each link and handling keyboard events. -
Consider adding an aria-label to each link for better screen reader context.
Here's an example of how to implement these changes:
<template>
<nav
class="fixed bottom-[3rem] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex max-h-[80vh] overflow-y-auto"
aria-label="Social media links"
role="navigation"
>
<a
href="https://discord.gg/WxXW9Jvv6k?ref=frontend.mu"
class="text-[#5865f2] text-sm md:text-normal hover:animate-bounce opacity-80 hover:opacity-100"
rel="noopener noreferrer"
target="_blank"
+ tabindex="0"
+ aria-label="Join our Discord community"
+ @keydown.enter="$event.target.click()"
>
<IconDiscord class="w-6" />
+ <span class="sr-only">Discord</span>
</a>
<!-- Apply similar changes to other social media links -->
</nav>
</template>
+<script>
+export default {
+ name: 'LeftMenu',
+ // Add any necessary component logic here
+}
+</script>These changes will significantly improve the accessibility and SEO of your social media navigation menu.
Committable suggestion was skipped due to low confidence.
| <template> | ||
| <nav | ||
| class="fixed bottom-[50px] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex" | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving responsiveness and accessibility of the navigation menu
The overall structure of the navigation menu is good, but there are a few areas for improvement:
-
Instead of using a fixed pixel value for the bottom offset (
bottom-[50px]), consider using a responsive unit likeremor a percentage. This will ensure better consistency across different screen sizes. -
Add appropriate ARIA attributes to improve accessibility. For example:
<nav
- class="fixed bottom-[50px] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex"
+ class="fixed bottom-[3rem] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex"
+ aria-label="Social media links"
+ role="navigation"
>- Consider adding a max-height to the menu and making it scrollable for very tall screens:
<nav
- class="fixed bottom-[3rem] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex"
+ class="fixed bottom-[3rem] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex max-h-[80vh] overflow-y-auto"
aria-label="Social media links"
role="navigation"
>These changes will improve the responsiveness and accessibility of your navigation menu.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <template> | |
| <nav | |
| class="fixed bottom-[50px] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex" | |
| > | |
| <template> | |
| <nav | |
| class="fixed bottom-[3rem] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex max-h-[80vh] overflow-y-auto" | |
| aria-label="Social media links" | |
| role="navigation" | |
| > |
| <a | ||
| href="https://discord.gg/WxXW9Jvv6k?ref=frontend.mu" | ||
| class="text-[#5865f2] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconDiscord class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://github.com/Front-End-Coders-Mauritius?ref=frontend.mu" | ||
| class="text-[#333] text-sm md:text-normal dark:text-white hover:animate-bounce" | ||
| > | ||
| <IconGithub class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://chat.whatsapp.com/invite/0kQ2QX0ZQ0j1YQ4X6Q4Q4Q" | ||
| class="text-[#25D366] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconWhatsappCommunity class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://twitter.com/frontendmu" | ||
| class="text-[#1DA1F2] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconTwitter class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://www.instagram.com/frontend.mu/?ref=frontend.mu" | ||
| class="text-[#d6249f] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconInstagram class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| href="https://www.linkedin.com/company/81846464/admin/?ref=frontend.mu" | ||
| class="text-[#007db1] text-sm md:text-normal hover:animate-bounce" | ||
| > | ||
| <IconLinkedin class="w-6" /> | ||
| </a> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance security, accessibility, and consistency of social media links
While the implementation of social media links is generally good, there are several improvements we can make:
-
Add
rel="noopener noreferrer"andtarget="_blank"attributes to each link for security when opening in a new tab. -
Consider adding a subtle visual indication that these are clickable links, such as a slight opacity change on hover.
-
Include visually hidden text for screen readers to provide context for each icon.
-
Consistently apply the
?ref=frontend.muquery parameter across all links.
Here's an example of how to implement these changes for the Discord link:
<a
href="https://discord.gg/WxXW9Jvv6k?ref=frontend.mu"
- class="text-[#5865f2] text-sm md:text-normal hover:animate-bounce"
+ class="text-[#5865f2] text-sm md:text-normal hover:animate-bounce opacity-80 hover:opacity-100"
+ rel="noopener noreferrer"
+ target="_blank"
>
<IconDiscord class="w-6" />
+ <span class="sr-only">Join us on Discord</span>
</a>Apply similar changes to all other social media links. Also, ensure that the ?ref=frontend.mu query parameter is consistently added to all links where applicable.
Committable suggestion was skipped due to low confidence.
| <img | ||
| class="w-16 aspect-square rounded-full" | ||
| :src="avatarUrl" | ||
| /> |
There was a problem hiding this comment.
Add alt attribute to avatar image for consistency and accessibility
While the reformatting of the avatar image element improves code readability, it's important to maintain consistency with the earlier implementation. To enhance accessibility and align with best practices, please add an alt attribute to this avatar image as well.
Consider applying the following change:
<img
class="w-16 aspect-square rounded-full"
:src="avatarUrl"
+ alt="User Avatar"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img | |
| class="w-16 aspect-square rounded-full" | |
| :src="avatarUrl" | |
| /> | |
| <img | |
| class="w-16 aspect-square rounded-full" | |
| :src="avatarUrl" | |
| alt="User Avatar" | |
| /> |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
packages/frontendmu-nuxt/components/icon/social/twitter.vue (1)
9-27: SVG implementation looks good with room for enhancement.The SVG implementation follows best practices with proper accessibility attributes and dynamic coloring. Consider these enhancements:
- Add aria-label for better accessibility
- Add width/height attributes for explicit dimensions
Apply this diff to enhance the implementation:
- <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"> + <svg + xmlns="http://www.w3.org/2000/svg" + viewBox="0 0 24 24" + width="24" + height="24" + aria-label="Twitter X Logo" + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/frontendmu-nuxt/components/icon/social/twitter.vue (1 hunks)
- packages/frontendmu-nuxt/components/site/LeftMenu.vue (1 hunks)
🔇 Additional comments (2)
packages/frontendmu-nuxt/components/site/LeftMenu.vue (2)
23-23: Verify WhatsApp community invite link.The WhatsApp invite link appears to be a placeholder. Please verify and update with the correct community invite link.
2-4: Review z-index value for potential overlay issues.The navigation uses
z-index: 30(z-30). Verify this value ensures proper stacking with other fixed/absolute positioned elements in the application.
| <script setup lang="ts"> | ||
| import { defineComponent } from "vue"; | ||
|
|
||
| defineComponent({ | ||
| name: "HugeiconsNewTwitterEllipse", | ||
| }); | ||
| </script> |
There was a problem hiding this comment.
Fix incorrect usage of defineComponent with script setup.
The defineComponent import and usage is incorrect here. When using <script setup>, you don't need to explicitly call defineComponent as components are automatically defined.
Apply this diff to fix the implementation:
<script setup lang="ts">
-import { defineComponent } from "vue";
-
-defineComponent({
- name: "HugeiconsNewTwitterEllipse",
-});
</script>If you need to explicitly set the component name for debugging purposes, use defineOptions instead:
<script setup lang="ts">
+defineOptions({
+ name: 'HugeiconsNewTwitterEllipse'
+})
</script>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <script setup lang="ts"> | |
| import { defineComponent } from "vue"; | |
| defineComponent({ | |
| name: "HugeiconsNewTwitterEllipse", | |
| }); | |
| </script> | |
| <script setup lang="ts"> | |
| </script> |
|
|
||
| <a | ||
| target="_blank" | ||
| href="https://www.linkedin.com/company/81846464/admin/?ref=frontend.mu" |
There was a problem hiding this comment.
Review LinkedIn URL format.
The LinkedIn URL contains "admin" in the path which might be incorrect for public access. Consider using the public company profile URL instead.
| <template> | ||
| <nav | ||
| class="fixed bottom-[50px] gap-4 flex-col pl-4 justify-end w-10 hidden xl:flex z-30" | ||
| > | ||
| <a | ||
| target="_blank" | ||
| href="https://discord.gg/WxXW9Jvv6k?ref=frontend.mu" | ||
| class="text-verse-500 dark:text-verse-200 text-sm md:text-normal hover:animate-bounce hover:ease-in-out hover:delay-200" | ||
| > | ||
| <IconDiscord class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| target="_blank" | ||
| href="https://github.com/Front-End-Coders-Mauritius?ref=frontend.mu" | ||
| class="text-verse-500 dark:text-verse-200 text-sm md:text-normal hover:animate-bounce hover:ease-in-out hover:delay-200" | ||
| > | ||
| <IconGithub class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| target="_blank" | ||
| href="https://chat.whatsapp.com/invite/0kQ2QX0ZQ0j1YQ4X6Q4Q4Q" | ||
| class="text-verse-500 dark:text-verse-200 text-sm md:text-normal hover:animate-bounce hover:ease-in-out hover:delay-200" | ||
| > | ||
| <IconWhatsappCommunity class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| target="_blank" | ||
| href="https://twitter.com/frontendmu" | ||
| class="text-verse-500 dark:text-verse-200 text-sm md:text-normal hover:animate-bounce hover:ease-in-out hover:delay-200" | ||
| > | ||
| <IconSocialTwitter class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| target="_blank" | ||
| href="https://www.instagram.com/frontend.mu/?ref=frontend.mu" | ||
| class="text-verse-500 dark:text-verse-200 text-sm md:text-normal hover:animate-bounce hover:ease-in-out hover:delay-200" | ||
| > | ||
| <IconInstagram class="w-6" /> | ||
| </a> | ||
|
|
||
| <a | ||
| target="_blank" | ||
| href="https://www.linkedin.com/company/81846464/admin/?ref=frontend.mu" | ||
| class="text-verse-500 dark:text-verse-200 text-sm md:text-normal hover:animate-bounce hover:ease-in-out hover:delay-200" | ||
| > | ||
| <IconLinkedin class="w-6" /> | ||
| </a> | ||
| </nav> | ||
| </template> |
There was a problem hiding this comment.
Add script section with component definition and icon imports.
The component is missing the script section. Add it to properly define the component and explicitly import the icon components:
<script>
import { defineComponent } from 'vue'
import IconDiscord from './icons/IconDiscord.vue'
import IconGithub from './icons/IconGithub.vue'
import IconWhatsappCommunity from './icons/IconWhatsappCommunity.vue'
import IconSocialTwitter from './icons/IconSocialTwitter.vue'
import IconInstagram from './icons/IconInstagram.vue'
import IconLinkedin from './icons/IconLinkedin.vue'
export default defineComponent({
name: 'LeftMenu',
components: {
IconDiscord,
IconGithub,
IconWhatsappCommunity,
IconSocialTwitter,
IconInstagram,
IconLinkedin
}
})
</script>
Linked to issue #195
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes