-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[ES|QL] Query History Design Review #180220
Comments
Pinging @elastic/kibana-esql (Team:ESQL) |
@MichaelMarcialis thanx a lot for your review. I see comments that are irrelevant with the change query history though 🤔 Does it make sense to split them into another issue? |
Adding some comments:
This is irrelevant with the query history changes. Let's create an ER for this if possible to discuss. I personally like that the change that is not going to be applied is disabled tbh
I would like to avoid adding more custom styles in this table which already has plenty of custom styles and it was very difficult to make it responsive 🙏
This Time ran column was the most difficult think to implement tbh. Changing to relative adds more complexity. I will create an ER for this but not sure how easy it is to tackle this
This is a bug, but has not been created by the query history. I will create a bug issue to track this,
This is a big change, I can create an ER for this. So with that being said I am going to clean up this issue a bit to enhancements, bug fix irrelevant with history, design review irrelevant with history. I will leave here only the comments that are related with the query history component. |
Based on my comments above I created the following issues:
I leave here the design review for query history, I will take care of it but due to the offsite it might not make it for 8.14. |
I am ticking the things that I have done in this PR #180579, it doesnt mean that they are merged :D |
@stratoula: Thanks for the creating those separate issues. As promised in the original issue comment, I've put together some quick high-fidelity mockups as a visual reference to accompany this design review. In addition to the points mentioned above, these mockups also suggest the following key changes:
Compact Experience Expanded Experience Inline Experience |
ES|QL History Design Review
Hey, @stratoula. Thanks so much for putting together the ES|QL run query history PR! As it merged while I was out on PTO, I wanted to give it a design test drive. It works great! Well done, as usual.
For my review, I've put together some initial design notes below. Let me know if they make sense or if they warrant further discussion. In the mean time, I will attempt to quickly mock up these suggestions in Figma to give you have some visual reference for what I'm suggesting below. Stay tuned.
CCing @andreadelrio as well, in case she has any other issues to add.
General Experience
The status icons are currently 12x12px. Can we increase them to the default 16x16px size?
Can we add tooltips for the status icons to indicate if each query was run successfully, or with warnings, or with errors?
The conditional presence of the accordion icon buttons causes any subsequent query text to be misaligned with any other row in the history table that doesn't have an accordion icon button. The hope was that the accordion buttons if/when present would occupy their own space in the column, allowing all text to remain vertically aligned. Would it be possible to implement such a change?
It appears that the current accordion icon buttons for truncated queries do not retain the standard icon button square ratio. Would it be possible to fix it so the icon buttons retain their correct ratio?
The conditional accordion icon buttons are using the
primary
blue color. Can we change this to thetext
gray color instead?Query text currently appears in the default EUI sans-serif font (Inter). Would it be possible to use our monospaced font for the query instead? Perhaps the code block component could be used here (for both stylistic and semantic purposes)?
The line height for multi-line queries appears a bit excessive at the moment. Would it be possible to tighten up the line height to more closely align with what we use for our monospaced code blocks in EUI?
Change the run button's
playFilled
icon toplay
.Change the copy-to-clipboard button's
copy
icon tocopyClipboard
.I believe it's now possible to change the text in the super date picker's update button. Per our previous slack conversation, can we change the "Update" text to be "Run"? The alternative "Refresh" text can stay as is.
Compact Experience
The bottom border of the editor's footer no longer appears when recent queries are hidden.
When recent queries are being shown, there appears to be a 2px border between editor footer and history table. This should be 1px.
When recent queries are being shown, there appears to be a 2px border when scrolled to the bottom of the table (likely due to the table's last row bottom border). Would it be possible to remove the last table row's border to fix this?
The "Last duration column" contents should be right aligned.
Expanded Experience
The accordion toggle buttons for larger, truncated queries don't appear in expanded editor mode initial. They only appear when the user goes back to compact mode, then returns to expanded mode. At that point, the truncated queries will correctly show the toggle buttons. Can we fix this bug?
The "Last duration column" contents should be right aligned.
Inline Experience
The "Submit feedback" and "Show recent queries" icon buttons don't appear to have any accompanying tooltips. Would it be possible to add some to better match the wireframes?
There appears to be a 2px border between editor footer and history table. This should be 1px.
There appears to be a 2px border between the history table and visualization configuration. This should be 1px.
The text was updated successfully, but these errors were encountered: