-
Notifications
You must be signed in to change notification settings - Fork 112
data explorer: convert to code UI update #8829
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
Conversation
E2E Tests 🚀 |
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.
Left some comments around the styling of the modal. I also think there's another small css bug. Let me know if you need some help looking into it!
.code-syntax-heading { | ||
margin-block-start: 0; | ||
margin-block-end: 8px; | ||
margin: 0 0 0 0; | ||
} |
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.
I don't think you want to set margin
and margin-block
here together. The margin-block
properties will set the margin
(margin-top
, margin-bottom
, margin-left
, and margin-right
) values for you based off the writing mode/text orientation.
You are also resetting the margin bottom value from 8px to 0px with margin: 0 0 0 0;
which I don't think you want. You can notice in the screenshot above that there is no margin between the dropdown header and syntax dropdown.
If you want to use margin
instead of margin-block
, the equivalent would be:
.code-syntax-heading {
margin-top: 0;
margin-bottom: 8px;
}
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.
ah, that makes sense! updated to only have margin top and bottom 👍
src/vs/workbench/browser/positronModalDialogs/convertToCodeModalDialog.tsx
Outdated
Show resolved
Hide resolved
.convert-to-code-editor { | ||
height: 200px; | ||
width: 100%; | ||
border: 1px solid var(--vscode-positronModalDialog-border); | ||
} |
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.
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.
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.
ah ya, that makes sense since the text content is nested deeper in the CodeEditorWidget
. We probably need to find the div the text is contained in and add the padding there.
> | ||
<VerticalStack> | ||
<ContentArea> |
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.
I don't know if this is reproducible for you, but I am able to scroll the entire contents of the modal by a few pixels to the left and right but there's no scrollbar showing up.
Screen.Recording.2025-08-05.at.1.39.48.PM.mov
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.
There is an invisible scrollbar in the CodeEditorWidget, but adjusting the width to be slightly smaller removed the wiggliness in the modal. There's definitely something weird going on with the sizing, it seems like the element is not actually the size of the border (see the padding comment/image)?
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.
I'll see if I can poke around and figure out what is going and how we can fix these issues!
…alDialog.tsx Co-authored-by: Dhruvi Sompura <[email protected]> Signed-off-by: Isabel Zimmerman <[email protected]>
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 great! I had one question but not a blocker for merging. 🚀
src/vs/workbench/browser/positronModalDialogs/convertToCodeModalDialog.css
Show resolved
Hide resolved
Signed-off-by: Isabel Zimmerman <[email protected]>
UI updates to the "Convert to Code" button!
Release Notes
New Features
Bug Fixes
QA Notes
with the
"dataExplorer.convertToCode": true
settingand then click on the
Convert to Code
button. See syntax highlighting.