-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
feat(Add) : Add Settings Toggle for Pause confirmation dialogue #6148
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks mostly good.
The issue you linked is wrong, or at least I am failing to see the relevance.
Also I would like to see this more properly integrated into the settings, so this first of all is persistent across devices and secondly users can uncheck it
src/components/PauseConfirm.vue
Outdated
/** Style of button */ | ||
btnStyle: { | ||
type: String, | ||
default: "btn-primary", | ||
}, | ||
/** Text to use as yes */ | ||
yesText: { | ||
type: String, | ||
default: "Yes", | ||
}, | ||
/** Text to use as no */ | ||
noText: { | ||
type: String, | ||
default: "No", | ||
}, | ||
/** Title to show on modal. Defaults to translated version of "Confirm" */ | ||
title: { | ||
type: String, | ||
default: null, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please only add props which you are actually using, unused features create bugs
@CommanderStorm i have made the changes, please have a look |
Hi @CommanderStorm, I have reverted back to Confirm.vue and deleted the PauseConfirm and extra logic that was added before, simply added a checkbox to Confirm.vue with a flag that becomes true when pause button is clicked |
How does this fit into the confirm component? Also: are you a human? Your answers and decisions se m very LLM-Like |
@CommanderStorm i am human , the reason i did was because i thought that this would make the workflow more simple as confirm and pauseConfirm had 80% similar fields but i admit this was a mistake The Confirm component now has two distinct responsibilities: |
This comment has been minimized.
This comment has been minimized.
Ah, thanks for the explanation. So if you are learning, you might not know about this:
So we can just reuse that work without either duplicating this component or fitting the change in there. |
Hi @CommanderStorm , can you take a look at the code again, i tried to implement your instructions, i think that this might work and also at ConfirmwithDonotShowAgain line 6 i have kept @yES="handleYes" |
Removed the checkbox for 'Do not show this again' from the modal.
Removed 'showCheckbox' prop and 'doNotShowAgain' data property from Confirm component.
I am getting a bit annoyed. Have you tested that your code works? |
Add "Do not show again" checkbox to pause confirmation dialogs
Closes #5234
📋 Overview
What problem does this pull request address?
What features or functionality does this pull request introduce or enhance?
PauseConfirm
component with enhanced functionalityResolves: User experience improvement for pause operations
Feature type: UI Enhancement
🛠️ Type of change
📄 Checklist
🎯 Functionality
User Flow:
Component Structure:
🧪 Testing
Manual Testing Performed:
Browser Compatibility: