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

Add delete experiences #36

Merged
merged 13 commits into from
Nov 10, 2023
Merged

Add delete experiences #36

merged 13 commits into from
Nov 10, 2023

Conversation

NitBravoA92
Copy link
Collaborator

🚩Required improvements in the React App

Hi @Dachrono and @AndreaM2429

We're sending this pull request for you to review my code. Thanks in advance for your feedback!

Milestones 🎯:

📌 Update Component functions to use ES6 arrow functions

📌 Create the AddExperience component with the form to add an experience

📌 Create the DeleteExperience component to display all the experience and allows the user to remove them

📌 Update the NavBar and the app routes to add two new links: /experiences/new and /experiences/delete

📌 Refactor the functionality to LogOut

⭐To the code reviewer 👨‍💻

Please, I kindly ask you to review the entire code and verify that all requirements have been met correctly.
if there is any issue 🦯 in this PR, please do list 📃 them in a descriptive 💡 manner and give your best suggestions 🎁 if needed.

@NitBravoA92 NitBravoA92 added the documentation Improvements or additions to documentation label Nov 9, 2023
@NitBravoA92 NitBravoA92 linked an issue Nov 9, 2023 that may be closed by this pull request
Copy link
Collaborator

@AndreaM2429 AndreaM2429 left a comment

Choose a reason for hiding this comment

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

Status: Approved 🟢 💯

Hi @HFG43 & @NitBravoA92

Great work on implementing the missing feature, this is complete so it's time to merge it 💯 🥳.

congrats

Highlights 🥇

  • Linters running
  • The app allows to create and delete experiences
  • Good design

Optional

All the changes with optional tags are not crucial enough to prevent you from getting the approval. Still, we would highly recommend that you consider these changes.

Happy coding! 👏 👏 👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear. Remember to tag me in your question so I can receive the notification.

<input id="testing" type="number" placeholder="Additional Testing Fee" className="form-input reserv-input" required min="0.01" step="0.01" />
<input id="fulltable" type="number" placeholder="Additional Full Table Fee" className="form-input reserv-input" required min="0.01" step="0.01" />
<input id="guests" type="number" placeholder="Guests included in Experience" className="form-input reserv-input" required min="1" step="1" />
<input id="image" type="text" placeholder="Add an Experience image" className="form-input reserv-input" required />
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Optional] You did an excellent job on the feature, I suggested you use a more descriptive placeholder on the image input, specifying that it is a URL image.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AndreaM2429 Thank you so much for giving us this valuable suggestion! The changes have been applied.

@NitBravoA92 NitBravoA92 merged commit b43929e into dev Nov 10, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[2.0] - Required improvements - Group Task
3 participants