search

SE310-1 / W.A.K

W.A.K is a movie tracker web-app developed to simplify and enhance the movie-rating experience while making this process more fun and customisable.
MIT License
2 stars 12 forks source link

Skau811 favlist custom order frontend #88

Closed skau811 closed 11 months ago

skau811 commented 11 months ago

This PR closes #53 by completing the last task of ordering the favourites list in a custom order.

Details of Changes:

Front-end Adjustments: Updated the current interface components to be compatible with the updated favourites list data structure. Ordering Feature: Implemented front-end allowing users to manually order their favourites. Also an attempt has been to update theme customisation feature to ensure the background-image changes according to first movie in the list.
Minor Styling: Incorporated minor CSS enhancements related to the favourites functionality to improve the user experience.

_Note for contributors who have already cloned the repo, make sure to run npm run installAll after pulling from main, so that dependencies can be easily updated._

Testing/Screenshots are as follows:

Favourites list with the sort-by drop-down menu image

After reording by top rated: image

Other minor CSS changes: Add to favourite button (The use of icons are better way of giving feedback to users instead of only a label text change) image

Favourites page when the list is empty: image

skau811 commented 11 months ago

Hi Sukhleen, thanks for adding these changes in. From my testing, the features match up with what you've described, so we're all good on that end! Unfortunately, I did actually ran into a small issue on my end (seen in the screenshot below) when trying to run the project to test it.

image

It was an easy fix, as once I ran npm run installAll, I no longer encountered the error. Merging this PR shouldn't be an issue for people who are cloning the repo for the first time, as they'll be running npm run installAll when they set it up anyway. But for people who have already cloned the repository, I'm thinking that perhaps if you could add a small note in the PR and/or a tag on it (or any other method you deem suitable) to inform people who have already cloned the repo to make sure they execute npm run installAll in the terminal, then it should prevent other contributors from running into this issue. Once this is done, I'd definitely be happy to merge it in! Outside of this small import issue, the PR itself is excellent, and adds some awesome functionality and styling to enhance our website - thank you very much!

Hi Vishva, Thanks for your review and highlighting the dependency issue that might happen if existing contributors pull from the main. As requested, I've updated the PR description with a note about running npm run installAll. I have also created a new label for the same and added it to my PR. This should guide other contributors and prevent future issues. Thanks again for your feedback, it is highly appreciated.