Skip to content

Conversation

@BartoszSocki
Copy link
Collaborator

No description provided.

@netlify
Copy link

netlify bot commented Dec 18, 2022

Deploy Preview for sparkledge ready!

Name Link
🔨 Latest commit 42ea5ea
🔍 Latest deploy log https://app.netlify.com/sites/sparkledge/deploys/63a7b5aa89cd8b00082bad19
😎 Deploy Preview https://deploy-preview-38--sparkledge.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@SKupisz SKupisz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. styled/subcomponents/navbar.tsx:
  • generally:
    a) min-width instead of max-width
    b) we use only vh when it comes to the height of components
    c) try to avoid "the divology" - it's not bad, but could be better
  • NavbarItem > hover without additional colour, let's leave filter brightness there
  • NavbarItemWrapper > RWD stuff goes at the end of a component
  • NavbarProps > put it on the top of the file
  • NavbarContainer > z-index should be constant and unchangeable
  1. Welcome component
    Something goes wrong with the filter after the changes > visible in the light mode

  2. Navbar in components/subcomponents/navbar.tsx

  • Opening and closing could be perhaps better with the usage of framer-motion
  • extract rendering of the specific groups of elements to separate files or make it as an array and then map it > the component is pretty long and the maintainability could be a bit harsh
  • More icons, less text in the navbar's items
  • state variables > missing the type of the variable after the useState and after useLocalStorage

Apart from that, everything looks good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants