[ES|QL] Improves the authoring of the query and charts #186875

stratoula · 2024-06-25T10:09:54Z

Summary

It makes the authoring of the ES|QL query easier by:

changing the flyout to be resizable
displaying a tab with the results of the query in the ES|QL datagrid component.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

andreadelrio · 2024-07-05T17:15:29Z

Hi @stratoula, a very nice addition to our ES|QL Dashboard UX here! As requested by @teresaalvarezsoler here's a first round of design feedback I wanted to share.

a) I don’t think we should be offering the entire power of Discover in such a small area. Therefore, I suggest we provide a simplified version of Discover here.

Remove the checkboxes and with them the Selected and Compare buttons and related functionalities

Offer a link for users to take their query and see it in the full Discover app if they wish to do so. Label it Open in Discover. See:

Given the limited space, I suggest we set Single as the default setting for Cell row height.
For the Pagination, given the limited space where we are displaying the results I think we should allow 25 rows per pages at the most. So we’d have Rows per page = [10, 25].

b) If I make any changes to the table, including resizing the columns, and then I edit and re-run the query, my changes to the table should not be altered. This is not the behavior currently. See:

Keep table settings (columns width, Cell row height, etc) intact upon editing and re-running the query

c) Other suggestions and/or findings

Just like in Discover, make the table’s header sticky
Similar to what we do for the Suggestions accordion, add a counter to the Results accordion
Consistently show the Columns button. Right now, sometimes it disappears. See:

stratoula · 2024-07-08T06:08:08Z

@andreadelrio thanx, not sure when I will find the time to work with this PR again but some comments:

If I make any changes to the table, including resizing the columns, and then I edit and re-run the query, my changes to the table should not be altered.

This is true but as the same happens for viz changes I suggest to tackle them together. (I have a task for this in 8.17)

Consistently show the Columns button. Right now, sometimes it disappears. See:

My guess is that this is how Discover works in small spaces but I will take a look

jughosta · 2024-07-09T09:31:57Z

Consistently show the Columns button. Right now, sometimes it disappears.

That's a part of EuiDataGrid logic for smaller screens, but we could override it in UnifiedDataTable if necessary

kibana/packages/kbn-unified-data-table/src/components/custom_toolbar/render_custom_toolbar.tsx

Line 51 in 339379e

const buttons = hasRoomForGridControls ? (

…fix'

stratoula · 2024-07-22T07:17:03Z

@andreadelrio thanx a lot for your review

As I mention above I have addressed everything except from the b. This is something which also works like that currently for the vis configuration settings and we are going to handle it most possibly in 8.17.

One note: I added the Open In Discover link but next to the columns sort button as the Discover toolbar has already a setting to add a new control there. Are you ok with it? Otherwise I will need to create a new position and will make the component a little bit more complicated.

stratoula · 2024-07-29T06:23:11Z

Thanx @andreadelrio, yes is not at the scope of this PR (lovely idea btw). I am going to return to this PR hopefully this week. I am dealing with other tasks with greater priority so this is mostly a PR I am working on my free time. @andreadelrio can you create an issue with the above comment for the presentation team (I assume it is their territory) if there is not one already?

stratoula · 2024-07-29T08:46:18Z

@andreadelrio about this:

When checking this on my external monitor, I noticed that it looks like given the increased viewport height we are showing white space below the results. Can we get rid of it?
It is being added by the euiDataGrid__virtualized so it seems I can't get rid of it (unless someone else knows the eui datagrid better than me)

stratoula · 2024-07-29T08:57:06Z

@andreadelrio I addressed your comments, can you give another look? 🙏

@elastic/kibana-visualizations same, I addressed Marco's comments, I know his out but I would appreciate a followup review 🙇‍♀️

mbondyra · 2024-07-29T13:27:10Z

src/plugins/esql_datagrid/public/data_grid.tsx

@@ -47,7 +59,8 @@ const DataGrid: React.FC<ESQLDataGridProps> = (props) => {
  const [activeColumns, setActiveColumns] = useState<string[]>(
    (props.initialColumns || (props.isTableView ? props.columns : [])).map((c) => c.name)
  );
-  const [rowHeight, setRowHeight] = useState<number>(5);
+  const [rowHeight, setRowHeight] = useState<number>(props.initialRowHeight ?? 5);
+  const [rowsPerPage, setRowsPerPage] = useState(10);


this constant is used here and then again in line 212. Could we export it to the variable? The one above too:

const DEFAULT_INITIAL_ROW_HEIGHT = 5 const DEFAULT_ROWS_PER_PAGE = 10

Sure e5f0fdc

mbondyra · 2024-07-29T13:35:17Z

x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx

@@ -128,6 +135,30 @@ export function LensEditConfigurationFlyout({
    return () => s?.unsubscribe();
  }, [dispatch, output$, layers]);

+  useEffect(() => {


why is this needed here?

It is because you need to initialize it, otherwise the accordion is not visible, only runs in the initialization btw

mbondyra · 2024-07-29T13:36:39Z

x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx

@@ -479,6 +508,78 @@ export function LensEditConfigurationFlyout({
              />
            </EuiFlexItem>
          )}
+          {isOfAggregateQueryType(query) && canEditTextBasedQuery && dataGridAttrs && (


Is it worth to extract this to a component?

Hmmm, we don't do it for the other accordion items 🤔 I don't think we will gain a lot, on the contrary you will need to pass a lot of properties to the component. The ESQLDatagrid is already a reusable component.

Done here b64dc30. As I said you don't gain a lot but it is not bad either!

mbondyra

LGTM, tested in Chrome 👌🏼

andreadelrio

@stratoula 🚀 Amazing work here! Thanks for working on all the UI polish tasks. Just one nit, let's make the icon in the Open in Discover blue as well (right now it shows in teal and black). Approving now assuming you can take care of that.

Related task (improve Preview) is already being tracked here.
#187864 (comment)

stratoula · 2024-07-30T06:28:55Z

@andreadelrio thanx a lot for your help with the design here!

Done

stratoula · 2024-07-30T06:29:30Z

/ci

kibana-ci · 2024-07-30T07:59:34Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 63d5a5c

Failed CI Steps

Jest Integration Tests #7

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lens`	1489	1490	+1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/esql-utils`	59	61	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`esqlDataGrid`	116.6KB	117.9KB	+1.3KB
`lens`	1.5MB	1.5MB	+2.2KB
total			+3.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	416.0KB	416.2KB	+168.0B
`esqlDataGrid`	9.2KB	9.3KB	+103.0B
`lens`	50.2KB	50.4KB	+142.0B
total			+413.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/esql-utils`	63	65	+2

History

💛 Build #224398 was flaky 34a6b3d
💔 Build #224363 failed 4506321
💔 Build #224339 failed 5ddbcf3
💔 Build #224311 failed e5dc627
💚 Build #224270 succeeded d452b31
💛 Build #222816 was flaky 9ed274b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

vadimkibana

ES|QL code changes LGTM.

[ES|QL] Display ESQL grid of results and make the flyout resizable

373270a

stratoula and others added 12 commits July 16, 2024 09:27

Merge with main and resolve conflicts

da2ebf7

some of the review comments

84ec51a

Display a badge of values

b3b5b64

Make the header sticky

88fa89e

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

98981f0

…fix'

Merge branch 'main' into esql-inline-editing-grid

c9edb85

Open in Discover link in progress

f1ae181

Add Discover link

d6fac74

Internationalization

92fc6dc

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

89cb3e9

…fix'

Merge branch 'main' into esql-inline-editing-grid

8c5a287

Merge with main and resolve conflicts

6dcd334

Adds unit tests

e23ccd6

stratoula changed the title ~~[ES|QL] Display ESQL grid of results and make the flyout resizable~~ [ES|QL] Improves the authoring of the query and charts Jul 22, 2024

Open Discover link in new tab

9ed274b

stratoula marked this pull request as ready for review July 22, 2024 10:30

stratoula requested review from a team as code owners July 22, 2024 10:30

stratoula added 4 commits July 29, 2024 08:23

Merge branch 'main' into esql-inline-editing-grid

d452b31

Fix typo

8ff2e6b

Adds abortController

1e874dc

Nit picks

9353484

stratoula mentioned this pull request Jul 29, 2024

Refactor inline editing flyout to useReducer instead of useState #189351

Open

Usde promise all

40e282b

Address Andrea's comments

e5dc627

stratoula added 2 commits July 29, 2024 12:39

Merge branch 'main' into esql-inline-editing-grid

5ddbcf3

Fix FT

73fde7c

stratoula requested a review from a team as a code owner July 29, 2024 12:14

stratoula added 3 commits July 29, 2024 14:19

Add min width

4506321

Revert

e4d050f

Add min size

9a49502

mbondyra reviewed Jul 29, 2024

View reviewed changes

Add constants

e5f0fdc

mbondyra reviewed Jul 29, 2024

View reviewed changes

stratoula added 2 commits July 29, 2024 15:59

Move to another component

b64dc30

add useCallback

34a6b3d

mbondyra approved these changes Jul 29, 2024

View reviewed changes

andreadelrio approved these changes Jul 30, 2024

View reviewed changes

Make the icon blue

63d5a5c

vadimkibana approved these changes Jul 30, 2024

View reviewed changes

stratoula merged commit 1bf4fcd into elastic:main Jul 30, 2024
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ES|QL] Improves the authoring of the query and charts #186875

[ES|QL] Improves the authoring of the query and charts #186875

stratoula commented Jun 25, 2024 •

edited

Loading

andreadelrio commented Jul 5, 2024 •

edited by stratoula

Loading

stratoula commented Jul 8, 2024

jughosta commented Jul 9, 2024

stratoula commented Jul 22, 2024

stratoula commented Jul 29, 2024 •

edited

Loading

stratoula commented Jul 29, 2024

stratoula commented Jul 29, 2024

mbondyra Jul 29, 2024

stratoula Jul 29, 2024

mbondyra Jul 29, 2024 •

edited

Loading

stratoula Jul 29, 2024 •

edited

Loading

mbondyra Jul 29, 2024

stratoula Jul 29, 2024 •

edited

Loading

stratoula Jul 29, 2024 •

edited

Loading

mbondyra left a comment

andreadelrio left a comment •

edited

Loading

stratoula commented Jul 30, 2024

stratoula commented Jul 30, 2024

kibana-ci commented Jul 30, 2024

API count

vadimkibana left a comment

[ES|QL] Improves the authoring of the query and charts #186875

[ES|QL] Improves the authoring of the query and charts #186875

Conversation

stratoula commented Jun 25, 2024 • edited Loading

Summary

Checklist

andreadelrio commented Jul 5, 2024 • edited by stratoula Loading

stratoula commented Jul 8, 2024

jughosta commented Jul 9, 2024

stratoula commented Jul 22, 2024

stratoula commented Jul 29, 2024 • edited Loading

stratoula commented Jul 29, 2024

stratoula commented Jul 29, 2024

mbondyra Jul 29, 2024

Choose a reason for hiding this comment

stratoula Jul 29, 2024

Choose a reason for hiding this comment

mbondyra Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

stratoula Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

mbondyra Jul 29, 2024

Choose a reason for hiding this comment

stratoula Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

stratoula Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

mbondyra left a comment

Choose a reason for hiding this comment

andreadelrio left a comment • edited Loading

Choose a reason for hiding this comment

stratoula commented Jul 30, 2024

stratoula commented Jul 30, 2024

kibana-ci commented Jul 30, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

vadimkibana left a comment

Choose a reason for hiding this comment

stratoula commented Jun 25, 2024 •

edited

Loading

andreadelrio commented Jul 5, 2024 •

edited by stratoula

Loading

stratoula commented Jul 29, 2024 •

edited

Loading

mbondyra Jul 29, 2024 •

edited

Loading

stratoula Jul 29, 2024 •

edited

Loading

stratoula Jul 29, 2024 •

edited

Loading

stratoula Jul 29, 2024 •

edited

Loading

andreadelrio left a comment •

edited

Loading