Skip to content

Conversation

@andresmgot
Copy link
Contributor

Closes #81

The changes are:

  • Split the existing QueryEditor code in two, adding the component QueryEditorCode that is used as a Custom Variable editor.
  • Simplify the $__schema, $__table and $__column selectors. Before those selectors were based on the SchemaInfo class that I took from the TimeStream data source but that class was quite complicated and using old react hooks. I used that approach because it was the only way I found to make the onSave method work in the editor. See this issue for more details. The issue is now solved but IMO we should not even use the onSave hook, that should save the dashboard, not just run it. Now it's just using Suggestions as Athena does.

@andresmgot
Copy link
Contributor Author

reverting last commit until that's fixed at grafana/athena-datasource#69

Copy link
Contributor

@sarahzinger sarahzinger left a comment

Choose a reason for hiding this comment

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

🎉

Copy link
Collaborator

@sunker sunker left a comment

Choose a reason for hiding this comment

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

LGTM

The issue is now solved but IMO we should not even use the onSave hook, that should save the dashboard, not just run it.

+1 for that. If we could use F5, like in any other SQL editor, that would be nice.

@sunker
Copy link
Collaborator

sunker commented Oct 7, 2021

BTW shouldn't we move the macros to the left side of the editor? Using the same layout as in Athena

@andresmgot
Copy link
Contributor Author

BTW shouldn't we move the macros to the left side of the editor? Using the same layout as in Athena

That would help with standardization yes (even though for Redshift is only Table/Column atm, not like all the options that Athena has).

@andresmgot andresmgot merged commit 813c54c into main Oct 13, 2021
@andresmgot andresmgot deleted the refactorFE branch October 18, 2021 09:11
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.

Refactor

3 participants