Skip to content

Conversation

piyush-pawar828
Copy link
Contributor

Description

No cancel button showing when creating new user via new review
issue link - #3604

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Screencast.from.26-03-25.03.50.30.PM.IST.webm

Manual review steps

I've run all the tests to ensure that none of the existing tests breaking due to my changes.

Manual reviewer: please leave a comment with output from the test if that's the case.

@github-actions github-actions bot added the Fix label Mar 26, 2025
Copy link

qlty-cloud-legacy bot commented Mar 26, 2025

Code Climate has analyzed commit f7b701d and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@piyush-pawar828
Copy link
Contributor Author

@Paul-Bob please help reviewing PR

Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

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

Hi @piyush828-design 👋

Thanks for submitting this PR!

I noticed that the cancel button in your example links to "admin/resources/users" and triggers a GET request when clicked.

Let's make sure the button doesn't render as a link, and instead includes:

data: { action: 'click->modal#close' }

This will ensure the modal closes without making any network requests.

Thanks again!

@piyush-pawar828
Copy link
Contributor Author

piyush-pawar828 commented Mar 27, 2025

Hi @Paul-Bob ,
Thanks for your inputs. Could you please help me to understand where should I add data: { action: 'click->modal#close' }?
as I added it in render_back_button method but it is still triggering the endpoint.
Sorry if I've done anything wrong as I didn't work on hotwire that much.
Thanks!
EDIT: When I added data: { action: 'click->modal#close' } in render_back_button it actually redirects to "admin/resources/users" but before that it was redirecting to /admin/resources/reviews/new and triggers "admin/resources/users" but doesn't redirect.

@Paul-Bob
Copy link
Contributor

Could you please help me to understand where should I add data: { action: 'click->modal#close' }?

Yes, that data attribute should be added to the button that's rendered as the "Cancel" button, but only when it's inside the new form modal. In that context, it should not trigger any redirect or navigate to another page.

Here are a couple of places where similar logic is already in use, for reference:

as I added it in render_back_button method but it is still triggering the endpoint.
Sorry if I've done anything wrong as I didn't work on hotwire that much.
Thanks!

No worries at all, appreciate you looking into it! You're right, it should be added in render_back_button, but only for this specific case when it's being rendered within the modal form. It shouldn't apply globally in all contexts.

@piyush-pawar828
Copy link
Contributor Author

Updated, thanks @Paul-Bob :)

@Paul-Bob
Copy link
Contributor

Thanks @piyush828-design it's looking good!

I haven’t had a chance to test it yet, but the logic appears to be correct. I’ll give it a proper run-through soon and get back to you.

Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

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

I've pushed a small tweak to preserve the original lines and avoid over-refactoring the link logic.

Thank you for this contribution @piyush828-design!

@Paul-Bob Paul-Bob merged commit 4c47f65 into avo-hq:main Mar 28, 2025
19 of 20 checks passed
@piyush-pawar828
Copy link
Contributor Author

Thank you @Paul-Bob ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants