Structure, styles and code

[leandroinacio]

BEM + Bulma The website is currently 8 pages - I would suggest not include Bulma, looks like an overkill that could be handled with css grid? In Waves.vue you have a class using camel case naming (waveContainer), even though the rest of the project is using BEM.

404 page: It's missing. You need a layouts/404 and an error.vue file to implement it.

Tests: E2E - based on what I saw in cypress/e2e/playerUsage.cy.js, a lot of strings being repeated over and over. I suggest moving these to an object, so you could just call PAGE_A.FOOTER_LINK_ABOUT_US.

Vitest- I would use it only for unit testing - that means, only testing the basic features of each component. The test tests/pageLoads.spec.js is checking page load, which should actually be handled by the E2E tests.

Links: I recommend using NuxtLink instead of tags in this case. For SPAs and SSRs, enables client-side navigation and avoids full page reload, as well as prefetching under the hood when the link is in the viewport.

I'll continue checking more later

https://github.com/drapisarda/serenegrove/blob/master/components/BurgerMenu.vue#L25

I would avoid scope styles for static pages and small websites. Consider using it only for big apps, enterprise software or SPAs.

https://github.com/drapisarda/serenegrove/blob/master/components/BurgerMenu.vue#L11

Slots should be lower case (<slot> instead of <Slot>) Also would consider naming them, since you have two slots there.

BurgerMenu.vue:

Consider adding aria-expanded for better accessibility.

https://github.com/drapisarda/serenegrove/blob/master/components/BurgerMenu.vue#L43

Consider making this color a variable.

https://github.com/drapisarda/serenegrove/blob/master/components/BurgerMenu.vue#L6

You are using BEM, so this div should have the class name = burger-menu__sidebar or this sidebar div should be in it's own component (Sidebar.vue). Same applies to the "close-button" class.

[leandroinacio]

https://github.com/drapisarda/serenegrove/blob/master/components/Switch.vue#L9

I suggest making type, requirement and when possible, default values defined:

const props = defineProps({
  modelValue: {
    type: Boolean,
    required: true
  }
});

components/Switch.vue

This looks like a component that could be used more than once in the page (although you only used it once), I would avoid the hardcoded id. Make it a required prop to avoid possible duplicated IDs.

https://github.com/drapisarda/serenegrove/blob/master/components/Switch.vue#L10

To impress the code reviewers, I suggest adding TS types or JSDoc to the emitted events for better documentation:

defineEmits<{
  (e: 'update:modelValue', value: boolean): void
}>()

or

/**
 * Emits an update event for modelValue
 * @event update:modelValue
 * @type { boolean }
 */
defineEmits(['update:modelValue'])

components/Switch.vue

Make more variables for better reusability.

[drapisarda]

BEM + Bulma The website is currently 8 pages - I would suggest not include Bulma, looks like an overkill that could be handled with css grid? In Waves.vue you have a class using camel case naming (waveContainer), even though the rest of the project is using BEM.

404 page: It's missing. You need a layouts/404 and an error.vue file to implement it.

Tests: E2E - based on what I saw in cypress/e2e/playerUsage.cy.js, a lot of strings being repeated over and over. I suggest moving these to an object, so you could just call PAGE_A.FOOTER_LINK_ABOUT_US.

Vitest- I would use it only for unit testing - that means, only testing the basic features of each component. The test tests/pageLoads.spec.js is checking page load, which should actually be handled by the E2E tests.

Links: I recommend using NuxtLink instead of tags in this case. For SPAs and SSRs, enables client-side navigation and avoids full page reload, as well as prefetching under the hood when the link is in the viewport.

I'll continue checking more later

Done! Thanks!

[drapisarda]

https://github.com/drapisarda/serenegrove/blob/master/components/BurgerMenu.vue#L25

I would avoid scope styles for static pages and small websites. Consider using it only for big apps, enterprise software or SPAs.

https://github.com/drapisarda/serenegrove/blob/master/components/BurgerMenu.vue#L11

Slots should be lower case (<slot> instead of <Slot>) Also would consider naming them, since you have two slots there.

BurgerMenu.vue:

Consider adding aria-expanded for better accessibility.

https://github.com/drapisarda/serenegrove/blob/master/components/BurgerMenu.vue#L43

Consider making this color a variable.

https://github.com/drapisarda/serenegrove/blob/master/components/BurgerMenu.vue#L6

You are using BEM, so this div should have the class name = burger-menu__sidebar or this sidebar div should be in it's own component (Sidebar.vue). Same applies to the "close-button" class.

BurgerMenu was not used! I removed It but also placed aria-expanded="true" in its proper place on Navigation.vue

[drapisarda]

defineEmits

Nice. A question: defineEmits is defined in vue runtime core lib. Should I redefine the type or add the JSDoc?

[leandroinacio]

Nice. A question: defineEmits is defined in vue runtime core lib. Should I redefine the type or add the JSDoc?

You won't redefined the function, you will import it and define the types of the emits you have in each component. Like in the example: https://vuejs.org/guide/typescript/composition-api#typing-component-emits

This helps because when you or any other developer use the component and try to use the emit, they will get autocompletion and also get whatever data type being emitted validated (so no mistakes between variable types).