Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sweden's National Parks #41

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

HeleneWestrin
Copy link

Netlify link

https://swedensnationalparks.netlify.app/

Collaborators

Because I am traveling Thursday to Sunday this week I did the project on my own.

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

Really nice job this week! And what a nice idea for a library too ⛰️ Let's have a look at your files.

HTML/CSS

  • Nice structure of your HTML, good that you remembered the labels ⭐
  • Nice that you're using CSS variables, makes the styles more maintanable and consistent 👍

JavaScript

  • You’re doing an excellent job using objects and arrays! You've created an array of objects (nationalParksInSweden) to store the parks’ data, which aligns perfectly with the learning goals.
  • You made good use of the forEach function as well as sort and filter ⭐
  • Functions like renderParks, sortParks, and updateParksDisplay are well-defined and handle their responsibilities clearly. This makes the code more maintainable and easier to read.

Clean Code
Your using descriptive variable names and your code is structured logically into sections and functions. Good use of comments too, keep this up!

Amazing job! Especially nice to see your take on the search bar ⭐

********************************* */

// Select all filter buttons within the element with id "filter-region"
const filterButtons = document.querySelectorAll("#filter-region button");
Copy link
Contributor

Choose a reason for hiding this comment

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

Clever!

Copy link

@joheri1 joheri1 left a comment

Choose a reason for hiding this comment

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

In general, great looking project! We love the park theme, nice touch with the favicon. The site itself is easy to navigate and is a visually cohesive experience. The design is clean and consistent, it highlights the images.

HTML:

  • Good use of semantic HTML.

CSS:

  • We are impressed by the CSS, even though we had to look up what some of these things meant and what they were doing so we learned a lot while doing this code review.

JS:

  • Well-organized code with Global constant, DOM selectors, etc. in clearly labeled sections.
  • The comments were super helpful, especially for us learning code.
  • One suggestion: Keep the array with data in its file (preferably) or at the top of the code file. You can also delete the folders with images you are not using in your project.

We were all inspired by your code and learned a lot by reviewing it. We're glad that we had this opportunity to look at your code. Keep up the awesome work 😃

//Joyce, Liselotte, and Johanna

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