-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Terminal]: 'Duplicate' button should remain in disabled state until user don't select a profile radio control. #12056
Comments
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request - Settings > Add a new profile - Disable "Duplicate" button until a profile is selected ![Duplicate](https://user-images.githubusercontent.com/25966642/148303450-a084cd5f-7f1c-4de3-86bd-602b9336649e.gif) <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Should take care of #12056, once we can get a build to the a11y team (MAINTAINER EDIT: we unfortunately can't just say "closes #foo" for issues like this one, we need another team to validate the following build.) * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request - Settings > Add a new profile - Disable "Duplicate" button until a profile is selected ![Duplicate](https://user-images.githubusercontent.com/25966642/148303450-a084cd5f-7f1c-4de3-86bd-602b9336649e.gif) <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Should take care of #12056, once we can get a build to the a11y team (MAINTAINER EDIT: we unfortunately can't just say "closes #foo" for issues like this one, we need another team to validate the following build.) * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed (cherry picked from commit a766798)
hey @v-rpundir ! I would to contribute in this issue, how can i proceed ? |
@rinkydevi Sorry for the confusion here. This was actually already fixed in #12096 (and then we entirely changed this UI again in #12326 such that this was no longer an issue). There's just a bit of a delay between when a11y bugs are fixed and when we can close them out. I'll yank off the help wanted/easy starter tags, since that's confusing. @v-rpundir can we confirm that this one is no longer an issue? |
@zadjii-msft thank u for your response sir! |
@zadjii-msft Main issue is fixed but Note issue still repro. Note Issue: 'Save' and 'Discard' button should remain in disabled state until we don't make any changes. |
@v-rpundir Ah okay. Missed that bit. That's actually quite a bit more complicated than disabling this one button till the user makes a selection. The Terminal doesn't have a good way to check when the user has actually made any changes via the UI. Also, the button will technically write out their settings to the file if they hit save even without changes. (I could have sworn there was a specific issue tracking this laying around but all I could find was a single bulletpoint in the #6800 megathread). For tracking purposes, what do you think the best strategy is here?
I'd rather not have two separate issues tracked in the same thread, so I just want to clarify a bit. Thanks! |
@zadjii-msft Closing this bug as Main issue is fixed and logged new separate issue for Note #12424 |
Windows Terminal version
1.12.3472.0
Windows build number
10.0.22504.1010
Other Software
Test Environment:
OS: Windows 11 Version Dev (OS Build 22504.1010)
App: Windows Terminal Preview
Screen Reader: Narrator
Steps to reproduce
Repro Steps:
User Experience:
Users UX will not be good especially Screen Reader as nothing will happen when they activate 'Duplicate' button.
Attachments:
'Duplicate' button should remain in disabled state until user don't select a profile radio control..zip
Expected Behavior
'Duplicate' button should remain in disabled state until user don't select a profile radio control.
Actual Behavior
'Duplicate' button is in enabled state even when user don't select a profile radio control.
Note Issue: 'Save' and 'Discard' button should remain in disabled state until we don't make any changes.
The text was updated successfully, but these errors were encountered: