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

Do not control SqlEditor height #687

Merged
merged 7 commits into from
Feb 20, 2024
Merged

Conversation

mshustov
Copy link
Collaborator

@mshustov mshustov commented Feb 7, 2024

closes #684

There is a bug in logic that on every change in the editor calls saveChanges({expand:true}), which resets the rawSql value back to the original state.
Instead of fixing this bug, I checked how Grafana handles this logic itself. From this example, it looks like the built-in Grafana editor handles height updates inside CodeEditor component, so I removed the external logic to manage component height.

Changelog

Remove manual expansion of SqlEditor.

@mshustov mshustov requested a review from a team as a code owner February 7, 2024 11:24
Copy link
Contributor

@asimpson asimpson left a comment

Choose a reason for hiding this comment

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

👍 looks good to me, thanks!

Just one nit and I'll approve.

src/components/SqlEditor.tsx Outdated Show resolved Hide resolved
@mshustov mshustov requested a review from asimpson February 14, 2024 15:09
@mshustov
Copy link
Collaborator Author

@asimpson, did the last failing check happen due to a lack of access to credentials? Can we merge the PR?

@asimpson asimpson enabled auto-merge (squash) February 15, 2024 12:51
@asimpson
Copy link
Contributor

@mshustov Looks like it. The build passed though which is what I care about. Can you merge main and I can merge the PR?

@mshustov
Copy link
Collaborator Author

@asimpson could you merge?

@asimpson asimpson merged commit 409f765 into grafana:main Feb 20, 2024
14 of 15 checks passed
@mshustov mshustov deleted the issue-684 branch February 21, 2024 08:21
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.

Unexpected rollback of changes in SQL Editor
2 participants