Skip to content

[Discover] Support for runtime fields editor in mobile view#97416

Merged
majagrubic merged 5 commits intoelastic:masterfrom
majagrubic:discover-rte-3
Apr 20, 2021
Merged

[Discover] Support for runtime fields editor in mobile view#97416
majagrubic merged 5 commits intoelastic:masterfrom
majagrubic:discover-rte-3

Conversation

@majagrubic
Copy link
Copy Markdown
Contributor

@majagrubic majagrubic commented Apr 19, 2021

Summary

Follow-up of: #96762

The PR extracts index pattern management code to its own component so that it's available in both DiscoverSidebar and DiscoverSidebarResponsive, thus making it render in mobile view as well.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@majagrubic majagrubic added v8.0.0 v7.13.0 release_note:enhancement Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// Feature:Discover Discover Application labels Apr 19, 2021
@majagrubic majagrubic marked this pull request as ready for review April 19, 2021 12:06
@majagrubic majagrubic requested a review from a team April 19, 2021 12:06
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@majagrubic
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

Copy link
Copy Markdown
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, button is now also displayed in "mobile" view 👍 I've got 1 request, in the current layout the button looks lost, not to say lonely, shouldn't it look like the version for more horizontal screen space?

Discover_-_Elastic

@majagrubic
Copy link
Copy Markdown
Contributor Author

Fair point, was confused with Lens, cause they have it that way 😬 Will change it.

@majagrubic
Copy link
Copy Markdown
Contributor Author

majagrubic commented Apr 20, 2021

@kertal fixed! indeed it looks better:
Screenshot 2021-04-20 at 14 56 13

Copy link
Copy Markdown
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested locally in Chrome, Firefox, Safari. Works as expected. Thx for making it pretty! Hurray for completing the runtime fields editor task in Discover 🍾 !

useNewFieldsApi={true}
/>
);
expect(component).toMatchSnapshot();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: It would recommendable to create test for more specific cases like clicking on the button, context menu contains 2 menu items, ... however this is good cleanup work and can be done later on

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will do it in a follow-up PR.

@kertal
Copy link
Copy Markdown
Member

kertal commented Apr 20, 2021

btw. think it should be skipped for release notes, since it's part, follow up or completion of a bigger task

@majagrubic majagrubic added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Apr 20, 2021
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 347 348 +1

Async chunks

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

id before after diff
discover 407.3KB 409.4KB +2.1KB

History

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

@majagrubic majagrubic merged commit 9da3268 into elastic:master Apr 20, 2021
@majagrubic majagrubic deleted the discover-rte-3 branch April 20, 2021 16:02
majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Apr 20, 2021
…97416)

* [Discover] Add runtime fields editor to mobile view

* Add a unit test

* Fix typescript issues

* Fixing layout on mobile

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
majagrubic pushed a commit that referenced this pull request Apr 20, 2021
…97666)

* [Discover] Add runtime fields editor to mobile view

* Add a unit test

* Fix typescript issues

* Fixing layout on mobile

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.13.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants