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

feat(sqllab): Highlight and make actionable table name in the editor #29783

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

justinpark
Copy link
Member

@justinpark justinpark commented Jul 30, 2024

SUMMARY

Following #26096

To make the editor smart, this commit highlights actionable items (such as table names) among the tokens hovered over by the mouse. When cmd + click is pressed, additional data linked to the relevant metadata is displayed in a popup.

The first item implemented in this commit is table names. If the SQL content includes a table name with a schema (e.g., main.bart_lines) or if the text matches a table name from the list of tables in the currently selected schema, it is marked as actionable. When clicked, it displays table metadata (including columns) and a preview query in a popup, similar to the left panel.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

highlight-ace-editor-table-token.mov

TESTING INSTRUCTIONS

type a sql including a table name and then mouse over the table name string

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@justinpark justinpark requested review from rusackas and eschutho July 30, 2024 23:36
@dosubot dosubot bot added change:frontend Requires changing the frontend sqllab Namespace | Anything related to the SQL Lab labels Jul 30, 2024
@justinpark
Copy link
Member Author

/testenv up

@justinpark justinpark requested review from yousoph and kasiazjc July 31, 2024 16:22
@justinpark justinpark force-pushed the feat--add-table-tooltip-marker-on-sql-editor branch from 9e1a2ff to 80e4290 Compare July 31, 2024 16:28
@kasiazjc
Copy link
Contributor

@justinpark that looks nice!

I think I have only one comment about preview tab which is - can copy + search (instead of filter) match the pattern in samples in explore?
image
image

@justinpark
Copy link
Member Author

can copy + search (instead of filter) match the pattern in samples in explore?

@kasiazjc This preview result is output in the same way by reusing the existing preview result component.
Screenshot 2024-07-31 at 11 20 20 AM
Unfortunately, it is not compatible with the data format of the explore sample table, so it cannot be reused. Instead, how about modifying the existing SQL Lab preview result control to have a UI layout similar to the explore sample table?

@kasiazjc
Copy link
Contributor

can copy + search (instead of filter) match the pattern in samples in explore?

@kasiazjc This preview result is output in the same way by reusing the existing preview result component. Screenshot 2024-07-31 at 11 20 20 AM Unfortunately, it is not compatible with the data format of the explore sample table, so it cannot be reused. Instead, how about modifying the existing SQL Lab preview result control to have a UI layout similar to the explore sample table?

honestly, that would be amazing as I think this would declutter SQL Lab a little bit! Is it something you would be able to work on?

I am also wondering about discoverability of this feature, because it can be hard to find if you dont know what you're looking for. Maybe for example in the empty empty state for SQL we could have some kind of "default doc placeholder sql comment" (sorry for the name lol), that would list shortcuts and features like the one you are implementing? What do you think?

@justinpark
Copy link
Member Author

honestly, that would be amazing as I think this would declutter SQL Lab a little bit! Is it something you would be able to work on?

Sure. Since it's separate issue, I'll push a new PR for the change.

I am also wondering about discoverability of this feature, because it can be hard to find if you dont know what you're looking for.

Of course, I have been considering the discoverability. As shown in the screenshot below, similar to existing editors, we have highlighted certain clickable keywords to provide feedback. When the cursor hovers over them, a tooltip is provided, making it easier to find the functionality.

mouse-over-token-tooltip.mov

@kasiazjc
Copy link
Contributor

kasiazjc commented Aug 1, 2024

honestly, that would be amazing as I think this would declutter SQL Lab a little bit! Is it something you would be able to work on?

Sure. Since it's separate issue, I'll push a new PR for the change.

I am also wondering about discoverability of this feature, because it can be hard to find if you dont know what you're looking for.

Of course, I have been considering the discoverability. As shown in the screenshot below, similar to existing editors, we have highlighted certain clickable keywords to provide feedback. When the cursor hovers over them, a tooltip is provided, making it easier to find the functionality.

mouse-over-token-tooltip.mov

Makes sense, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:frontend Requires changing the frontend packages size/XXL sqllab Namespace | Anything related to the SQL Lab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants