Skip to content

fix(compass-schema): sticky copy button COMPASS-9141 #6804

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 19, 2025
Merged

fix(compass-schema): sticky copy button COMPASS-9141 #6804

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions packages/compass-schema/src/components/export-schema-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ const contentContainerStyles = css({
});

const codeEditorContainerStyles = css({
maxHeight: `${spacing[1600] * 4 - spacing[800]}px`,
overflow: 'auto',
height: `${spacing[1600] * 4 - spacing[400]}px`,
padding: spacing[100],
});

const codeStyles = css({
'& .cm-editor': {
Copy link
Member

@Anemy Anemy Mar 18, 2025

Choose a reason for hiding this comment

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

It does strike me as not the best practice to be overriding the internal styles of the editor here. Ideally compass-schema shouldn't know we're even using code mirror under the hood. Since we're already doing it, I think fine as is, but something we should probably avoid.

Copy link
Member

@Anemy Anemy Mar 18, 2025

Choose a reason for hiding this comment

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

I was the one who introduced this too 🤦
Looks like we do it in a few other places in Compass also.

Copy link
Collaborator Author

@paula-stacho paula-stacho Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You're right, we could've just added some parameters that the compass component would translate into these styles. We don't and we even named the component <CodeMirrorMultilineEditor />, so I think the ship has sailed 😅 . And by sailed I mean I don't think we have time to fix that atm 🙈

Copy link
Collaborator

@gribnoysup gribnoysup Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

... or you can just set maxLines on the component to 15 (which matches the math here) and it will already make the editor limited in size and the action buttons sticky with no extra styles and class names referring to the component internals 🙂

paddingLeft: spacing[2],
paddingLeft: spacing[200],
maxHeight: `${spacing[1600] * 4 - spacing[800]}px`,
},
'& .multiline-editor-actions': {
marginRight: spacing[300],
},
});

Expand Down
Loading