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

feat: added delete override modal #130

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

sauraww
Copy link
Collaborator

@sauraww sauraww commented Jun 19, 2024

Problem

The Delete button in the override feature was deleting overrides immediately, without requesting user confirmation. This behavior poses a risk as these overrides are sensitive and critical.

Solution

Implemented a confirmation modal that appears when the Delete button is clicked. This modal prompts the user to confirm whether they really want to delete the override. The override will only be deleted if the user confirms by selecting "Yes."

Here is the screeshot for the UI Review :

image

@sauraww sauraww requested a review from a team as a code owner June 19, 2024 09:13
@sauraww sauraww force-pushed the feat/delete-override-modal branch from 6ad101b to 842dc3e Compare June 19, 2024 09:16
@@ -326,6 +342,22 @@ pub fn context_override() -> impl IntoView {
}}

</div>

<Show when=move || modal_visible.get()>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sauraww Can you make this a separate module for future usage?

Copy link
Collaborator

Choose a reason for hiding this comment

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

And make the tailwind a bit cleaner to read by separating out common ones, hard to read those.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed on making this a component that can be quickly used since we have deletes in multiple screens

<dialog id="my_modal_2" class="modal" open={modal_visible.get()}>
<div class="modal-box bg-white rounded-lg p-6 shadow-xl border-2 border-lightgray">
<h4 class="text-xl font-semibold text-gray-800 mb-4">Confirm Deletion</h4>
<p class="text-sm text-gray-600 mb-6">Are you sure you want to delete this context? This action cannot be undone.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<p class="text-sm text-gray-600 mb-6">Are you sure you want to delete this context? This action cannot be undone.</p>
<p class="text-sm text-gray-600 mb-6"> Are you sure you want to delete this context? Action is irreversible.</p>

<h4 class="text-xl font-semibold text-gray-800 mb-4">Confirm Deletion</h4>
<p class="text-sm text-gray-600 mb-6">Are you sure you want to delete this context? This action cannot be undone.</p>
<div class="flex justify-end space-x-4">
<button class="btn bg-purple-500 text-white px-4 py-2 rounded hover:opacity-75 hover:bg-purple-500" on:click=confirm_delete>Yes, Delete</button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be rounded buttons and follow the same look we have for all buttons

@sauraww sauraww force-pushed the feat/delete-override-modal branch 2 times, most recently from c423efc to 49fad50 Compare July 2, 2024 12:45
@pratikmishra356 pratikmishra356 self-requested a review July 3, 2024 06:02
<p class="text-sm text-gray-600 mb-6">{header_text.clone()}</p>
<div class="flex justify-end space-x-4">
<button class="btn bg-purple-500 font-medium rounded-lg text-sm text-center text-white px-5 py-2.5 me-2 mb-2 hover:opacity-75 hover:bg-purple-500" on:click=move |_| confirm_delete.call(()) >Yes, Delete</button>
<button class="btn bg-gray-300 font-medium rounded-lg text-sm text-center text-black px-5 py-2.5 me-2 mb-2 hover:opacity-75 hover:bg-gray-300" on:click=move |_| set_modal_visible.set(false)>No </button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

except bg-color make a variable and use that will be cleaner.

@sauraww sauraww force-pushed the feat/delete-override-modal branch from 49fad50 to 3ba41b1 Compare July 3, 2024 06:58
@sauraww sauraww force-pushed the feat/delete-override-modal branch from 3ba41b1 to eb3974c Compare July 3, 2024 07:00
@mahatoankitkumar mahatoankitkumar merged commit a3d498e into main Jul 3, 2024
4 checks passed
@mahatoankitkumar mahatoankitkumar deleted the feat/delete-override-modal branch July 3, 2024 07:09
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.

4 participants