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

Webify - Final Project Web Dev Bootcamp 2024 #48

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

Conversation

SofieFerrari and others added 30 commits May 29, 2024 09:46
Co-authored-by: Sofie Ferrari Strahl <[email protected]>
Co-authored-by: Mai K <[email protected]>
Co-authored-by: Sofie Ferrari Strahl <[email protected]>
Copy link

@AntonellaMorittu AntonellaMorittu left a comment

Choose a reason for hiding this comment

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

Hello Mai, Sofie and Wen and grattis! 🎓

I am so impressed with the app you delivered, beginning from the clean design to the smooth flow you proved to be capable of developing and shipping web apps! I can see your planning was meticulous, from the wireframe to the design on Figma. You nailed aspects like responsiveness and positive user experience, the product is finalised and refined in many aspects, including animations, info states, a breadcrumb component, besides features like products gather by tags and authentication that take the app to a really high level.

The code structure is clean and the backend is robust, really good you divided your code in modules on the server-side 💯

Now that said, I spotted some few things that can be improved and little bugs to be squashed:

  • accessibility: should score at least 95 on Lighthouse (basic requirement)
  • upload SVG images rather than any other format to preserve high definition
  • if logged out, remove items from cart doesn’t work (clear cart does!)
  • delete all console.log() added during development on the frontend
  • UX: when selecting an option from footer, render the page by scrolling to the top, or the user won’t notice the selection has been applied
  • when selecting a category from the footer, the url is uploaded but not the content of the page

Again congratulations for shipping a final product in only 3 weeks! Thank you for reviewing and submitting the changes requested.

formState: { isValid }, // check if data fulfills the requirements
} = useForm();
const onSubmit = (data) => {
console.log(data);

Choose a reason for hiding this comment

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

please remove the console.log() used during development 🧹

@wwenzz
Copy link

wwenzz commented Jun 21, 2024

Hej @AntonellaMorittu we have adjusted our code based on your suggestions.

- accessibility: should score at least 95 on Lighthouse (basic requirement)
Now we have improved our accessibility score to 100.
Screenshot 2024-06-21 at 23 16 14

- upload SVG images rather than any other format to preserve high definition
We have cleaned up our images and converted the icons and smaller images to SVG. Yet we think larger images would be better in png/jpg format.

- if logged out, remove items from cart doesn’t work (clear cart does!)
This has been fixed and now logout clears the cart.

- delete all console.log() added during development on the frontend
All console logs have been removed in frontend folder.

- UX: when selecting an option from footer, render the page by scrolling to the top, or the user won’t notice the selection has been applied
We have implemented the scroll-to-top function that would work on the links in the footer

- when selecting a category from the footer, the url is uploaded but not the content of the page
We have fixed this issue and now the category from footer will execute filter function as well as modify the query params.

I hope what we have done meets your expectations and if anything missing please let us know.

Again, thank you so much for your detailed feedback and these help us improve our project a lot!

/Mai, Sofie and Wen

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.

4 participants