Skip to content

Review Notes #2

@tombenezra

Description

@tombenezra

Kudos! Well done! 👏
I know you haven't implemented it in "production ready" coding mode, but here's a few notes to acknowledge:

  • When applying margin, padding (any spacing basically), it should be done on the higher-level container/wrapper rather than the first/last child inside this container. for instance have a 10px spacing on the .main-right rather than the .menu-text label element would make more sense and support possible future changes
  • the idea of dividing the higher-level layout (header or body) by percentages could be problematic when changing screen size/layout shifts and will most probably make some areas non-responsive.
    this could be solved by letting flex do it's magic, for example the main container could be separated into:
    left side (nav) - 250px; middle (feed) - fill; right side (contacts) - 250px
    so it will stretch according to screen size, you could do that by giving the middle container {flex-grow: 1}. same applies for the header of course
  • some of the rules/attributes aren't applied at all or being overridden by other rules, consider removing them
  • separating the styles into different sections is a step in the right direction, however separating into multiple files can make the code even more organized and easy to navigate in
  • let the content of an element determine the dimensions, and avoid using height, width unless necessary. if you want to expand/shrink a specific element, prefer using padding/margin (again, if possible of course)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions