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): giving the query history pane a facelift #31316

Merged
merged 7 commits into from
Dec 12, 2024

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Dec 6, 2024

Fixing some styling and UI components in the query history pane in SQL Lab

  • using label with monospace for timestamps
  • set mouse to pointer on clickable icons
  • using consistent icon size
  • make the View clickable a button instead of label

before

Screenshot 2024-12-05 at 7 04 25 PM

after

Screenshot 2024-12-05 at 7 03 00 PM

@dosubot dosubot bot added the sqllab Namespace | Anything related to the SQL Lab label Dec 6, 2024
@@ -384,6 +384,7 @@ export function runQueryFromSqlEditor(
ctas,
ctasMethod,
) {
console.log('YOYO', queryEditor);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
console.log('YOYO', queryEditor);

.catch(() =>
dispatch(addDangerToast(t('Failed at stopping query. %s', query.id))),
);
.catch(() => dispatch(addDangerToast(t('Failed at stopping query.'))));
Copy link
Member

Choose a reason for hiding this comment

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

"Failed to stop query"?

@mistercrunch mistercrunch force-pushed the query-history-facelift branch from 7195636 to 700607d Compare December 6, 2024 21:22
@pull-request-size pull-request-size bot added size/L and removed size/M labels Dec 6, 2024
@kasiazjc
Copy link
Contributor

kasiazjc commented Dec 9, 2024

Looks clean @mistercrunch! Two things from me me:

  • icons seems big on comparison to the rest of the items in the row - what size is it?
  • I like pill for duration, but I think using it for timestamp might be too much - we have so many different items that it can get overwhelming and in other places we just use text for timestamp

Thoughts?

@mistercrunch
Copy link
Member Author

@kasiazjc -> screenshot based on latest tweaks
Screenshot 2024-12-09 at 10 28 53 AM

Thinking a bit more about it, I'd like to create a standard way to represent timestamps/durations through a reusable react component.

GPT-designer suggested this with a little clock which I think is kind of cute, but will make things more busy ->
Screenshot 2024-12-09 at 10 30 23 AM

My issue with out current CRUD list/table views is that it looks very raw (I mean that as opposed to "rich"), and would love to make it looking richer if that makes sense. Adding clock icons might look more busy, but also richer in some capacity.

@mistercrunch
Copy link
Member Author

/testenv up

Copy link
Contributor

github-actions bot commented Dec 9, 2024

@mistercrunch Processing your ephemeral environment request here.

Copy link
Contributor

github-actions bot commented Dec 9, 2024

@mistercrunch Ephemeral environment spinning up at http://44.243.18.92:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@kasiazjc
Copy link
Contributor

@kasiazjc -> screenshot based on latest tweaks Screenshot 2024-12-09 at 10 28 53 AM

Thinking a bit more about it, I'd like to create a standard way to represent timestamps/durations through a reusable react component.

GPT-designer suggested this with a little clock which I think is kind of cute, but will make things more busy -> Screenshot 2024-12-09 at 10 30 23 AM

My issue with out current CRUD list/table views is that it looks very raw (I mean that as opposed to "rich"), and would love to make it looking richer if that makes sense. Adding clock icons might look more busy, but also richer in some capacity.

I think it looks more balanced with smaller icons and the pills for timestamp and duration look better too with it. Looks good for now and let's figure out crud enrichment/details next!

@rusackas
Copy link
Member

image This progress bar misalignment is nagging at me... I'll see about a PR for that when I come up for air next :D

@mistercrunch
Copy link
Member Author

mistercrunch commented Dec 11, 2024

@rusackas addressed the comments, probably mergeable as net positive

@rusackas
Copy link
Member

LGTM!

@rusackas rusackas merged commit 4ff9aac into master Dec 12, 2024
34 checks passed
@rusackas rusackas deleted the query-history-facelift branch December 12, 2024 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preset-io size/L sqllab Namespace | Anything related to the SQL Lab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants