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

[Discover] Deangularize context.app #80851

Merged
merged 7 commits into from
Oct 21, 2020

Conversation

majagrubic
Copy link
Contributor

@majagrubic majagrubic commented Oct 16, 2020

Summary

Addresses: #70211

Moves doc-table part of context_app.html to React component and creates a new directive.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@majagrubic
Copy link
Contributor Author

@elasticmachine merge upstream

@majagrubic
Copy link
Contributor Author

@elasticmachine merge upstream

@majagrubic majagrubic marked this pull request as ready for review October 20, 2020 11:12
@majagrubic majagrubic requested a review from a team October 20, 2020 11:12
@majagrubic majagrubic added release_note:skip Skip the PR/issue when compiling release notes Feature:Discover Discover Application v7.11.0 labels Oct 20, 2020
Copy link
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.

Thanks for moving this old rock! Tested locally in Chrome, Safari, Firefox, works 👍, left some code comments and suggestions, one thing is mandatory i18n provider is missing leading to the following message in the browsers console:
Bildschirmfoto 2020-10-20 um 14 46 27

_version: 1,
_source: {
category: ["Men's Clothing"],
currency: 'EUR',
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Walker Text Ranger's first name ist Cordell and he would never pay in EUR !

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
discover 243 245 +2

async chunks size

id before after diff
discover 439.3KB 442.0KB +2.7KB

History

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

Copy link
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, thanks for also removing kui classes, checked it out, tested in Chrome, Firefox, Safari, works 👍

@majagrubic majagrubic merged commit 3df30e0 into elastic:master Oct 21, 2020
@majagrubic majagrubic deleted the deangularize-context branch October 21, 2020 12:36
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 21, 2020
…arm-phase-to-formlib

* 'master' of github.com:elastic/kibana: (55 commits)
  [UX] Fix map color variance and apply proper filter for extended stats (elastic#81106)
  [User Experience] Use EuiSelect for percentiles instead of SuperSelect (elastic#81082)
  [DOCS] Add link for monitoring ssl settings (elastic#81057)
  [test] Await loading indicator in monitoring test (elastic#81279)
  [ILM] Minor copy and link additions to cloud CTA for cold phase (elastic#80512)
  [Mappings editor] Add scaled_float and date_range comp integration tests (elastic#81287)
  [Discover] Deangularize context.app (elastic#80851)
  [O11y Overview] Add code to display/hide UX section when appropriate (elastic#80873)
  [Discover] Extend DiscoverNoResults component to show different message on error (elastic#79671)
  Fix tagcloud word overlapping (elastic#81161)
  [Security Solution] Fixes flaky test rules (elastic#81040)
  Changed the code to avoid tech debt with hacky solutions after receiving comments on EUI issue reported about this problem. (elastic#81183)
  [Security Solution][All] Replace old markdown renderer with the new one (elastic#80301)
  Add namespaced version of the API call (elastic#81278)
  [ML] Data Frame Analytics: Fix race condition and support for feature influence legacy format. (elastic#81123)
  [Fleet] Fix POLICY_CHANGE action creation for new policy (elastic#81236)
  [Security Solution][Endpoint][Admin] Malware user notification checkbox (elastic#78084)
  [SecuritySolution][Unit Tests] - fix flakey unit test (elastic#81239)
  skip flaky suite (elastic#81264)
  [Maps] fix top-level Map page is called 'Kibana' (elastic#81238)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared/forcemerge_field.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase.tsx
majagrubic pushed a commit that referenced this pull request Oct 22, 2020
* [Discover] Deangularize context.app

* Add a unit test

* Addressing PR comments; fixin I18n provider; replacing kui with EUI code

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 v7.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants