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

Fix dialog buttons #23843

Merged
merged 3 commits into from
Jul 13, 2023
Merged

Fix dialog buttons #23843

merged 3 commits into from
Jul 13, 2023

Conversation

Charles-Gagnon
Copy link
Contributor

@Charles-Gagnon Charles-Gagnon commented Jul 12, 2023

For #23827

In the latest VS Code merge they added a check to the label setter to no-op if the label is the same. But because we were clearing the label directly here by setting the innerHTML the button still thought it had the original text so never "refreshed" with the original text.

Before :

VirtualizeBroken

After :

VirtualizeFixed

SavingProperties

@@ -49,8 +49,6 @@
"sql/base/browser/ui/panel/panel.ts",
"sql/base/browser/ui/selectBox/selectBox.ts",
"sql/base/browser/ui/panel/panel.component.ts",
"sql/workbench/services/dialog/browser/dialogModal.ts",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went through the other files we were directly setting HTML in and didn't see anything else with this issue - they're mostly just creating raw HTML elements and directly setting the contents or other seemingly-valid use cases.

@Charles-Gagnon Charles-Gagnon merged commit d294c81 into main Jul 13, 2023
@Charles-Gagnon Charles-Gagnon deleted the chgagnon/buttons branch July 13, 2023 03:54
Charles-Gagnon added a commit that referenced this pull request Jul 13, 2023
* Fix dialog buttons

* comments

* Remove debug

(cherry picked from commit d294c81)
Charles-Gagnon added a commit that referenced this pull request Jul 13, 2023
* Fix dialog buttons

* comments

* Remove debug

(cherry picked from commit d294c81)
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.

3 participants