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

Display items homepage #24

Merged
merged 6 commits into from
Jun 27, 2023
Merged

Display items homepage #24

merged 6 commits into from
Jun 27, 2023

Conversation

PabloBona
Copy link
Owner

@PabloBona PabloBona commented Jun 27, 2023

Hello Partner !!

I have made some exciting enhancements to the project that greatly improve the show display and overall user experience. I would like to submit this pull request for your consideration.

Changes included in this pull request:
✅ Introduction of API Logic: The functionality has been refactored to introduce api.js and renderShows.js files. These files handle the API requests, retrieve the series data, and render the shows respectively. This modular approach improves code organization and maintainability.
✅ Addition of Like Button: I have added a heart-shaped button (❤️) to enable users to express their liking for a particular show to be implemented in future releases.
✅ Feedback Button: A feedback button (📝) has been introduced to collect feedback and feedback to be implemented in future releases
✅ Load Event Implementation: A load event has been implemented to display the show cards on page load. This provides a seamless and enhanced user experience, ensuring that the show cards are loaded and displayed promptly.

Add <div> for error handling
Implement load event to display cards on page load
Introduce api.js, getSeries.js, and renderShows.js
Update Dist
@PabloBona PabloBona temporarily deployed to github-pages June 27, 2023 13:53 — with GitHub Pages Inactive
@PabloBona PabloBona temporarily deployed to github-pages June 27, 2023 14:31 — with GitHub Pages Inactive
Copy link
Collaborator

@NitBravoA92 NitBravoA92 left a comment

Choose a reason for hiding this comment

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

Hi @PabloBona ,

GIF

You have really done an excellent job on this update!
Everything works perfectly fine and meets the requirements. There is nothing else to do other than... merge it :shipit:
Congratulations! 🎉

Highlights

✅ You used Gitflow correctly 👍
✅ No linter error 👍
✅ Good commit messages 👍
✅ Good descriptive PR 👍

✅ You did all the necessary implementations to display the series list 👍

✅ The HTML code of the Series cards is very clean and understandable. 👏

✅The API request returns the requested data correctly and the number of series has been limited to 12, that's great! 👏

✅ Excellent UI design. The entire website looks very attractive, the color scheme is perfect. The size of the elements is adequate. It is 100% responsive. Well done! 👏

Optional suggestions

_Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you take them into account as they can make your code better.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.

Comment on lines +12 to +19
try {
const response = await getShows();

if (!response.ok) {
throw new Error('Failed to fetch Series');
}

const shows = await response.json();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi, dear partner!! @PabloBona
Your code works as expected. Good job! 🎉🎉🎉

This is completely [OPTIONAL]:

The logic that resolves the entire promise (without the renderShows(shows); function call) could be encapsulated in a function and simply called within this event handler. This could improve the organization and future reuse of your code.

Keep up the excellent work! Your progress in this project is very good and valuable. 👏👏👏

@PabloBona PabloBona merged commit 03d3125 into dev Jun 27, 2023
6 of 7 checks passed
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.

None yet

2 participants