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

JAN22P #1105 delete listing as client #1114

Merged
merged 6 commits into from
Dec 16, 2024

Conversation

Nightfox77
Copy link

Title

#1105- Deleting a listing as a client

Describe your changes:

  • added text to the close modal button and made sure it is visible
  • changed the delete button click event
  • implemented headers function in the API call
  • created new event listeners and assigned them to the fitting buttons
  • added JSdocs

Checklist before requesting a review

  • I created a new branch before starting with this ticket?
  • I commented the issue after finish in the Frontend Team Board?
  • I moved the card to Quality Assurance after finish?

Feedback

  • Yes, I want feedback.

@Nightfox77 Nightfox77 requested a review from johnsulf December 12, 2024 17:54
@Nightfox77 Nightfox77 self-assigned this Dec 12, 2024
@Nightfox77 Nightfox77 added the QA Quality Assurance label Dec 12, 2024
@Nightfox77 Nightfox77 changed the base branch from main to JAN22P December 12, 2024 21:47
Copy link

@johnsulf johnsulf left a comment

Choose a reason for hiding this comment

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

QA Feedback

Hi Michael,

Great job on fixing the issue. I have verified the issue, and verified your fix. It works well!

I just have a few things I would like you to do before we merge the PR.

  1. Add button classes to the close button so it fits better with the design. For example:
<button 
  id="hide-delete-modal-btn" 
  class="btn btn-theme-dark rounded-0"
  data-bs-dismiss="modal"
  data-bs-target="#deleteListingModal">
    Close
</button>
  1. The cancel-delete-listing-btn also deletes the listing. Maybe we should fix that while we are in this area. It could just close the modal or something.

@Nightfox77 Nightfox77 requested a review from johnsulf December 16, 2024 12:41
Copy link

@johnsulf johnsulf left a comment

Choose a reason for hiding this comment

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

Great! Looks really good! I will merge the changes to JAN22P and delete this branch.

  • Erlend

@johnsulf johnsulf merged commit 9bfeb28 into JAN22P Dec 16, 2024
@johnsulf johnsulf deleted the JAN22P-#1105-delete-listing-as-client branch December 16, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JAN22P QA Quality Assurance
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

As client(company)- Listing gets deleted before delete button in modal is clicked
2 participants