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] make field icons consistent across field list and doc tables #129621

Conversation

drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented Apr 6, 2022

Summary

Fixes #124017

Makes icons in the doc viewer tables match the fields list.

Modern

Screen Shot 2022-04-06 at 12 31 28 PM

Legacy

Screen Shot 2022-04-06 at 10 52 01 AM

Testing help

1. Create an index with a version field and give it a document

PUT test_icons
{
  "mappings": {
    "properties": {
      "my_version": {
        "type": "version"
      }
    }
  }
}

POST test_icons/_doc
{
  "my_version": "1.5"
}

2. Create data view

3. View the document in Discover

@drewdaemon drewdaemon added Feature:Discover Discover Application release_note:fix auto-backport Deprecated - use backport:version if exact versions are needed Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.2.0 v8.3.0 labels Apr 6, 2022
@drewdaemon drewdaemon marked this pull request as ready for review April 6, 2022 17:41
@drewdaemon drewdaemon requested a review from a team as a code owner April 6, 2022 17:41
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@drewdaemon drewdaemon changed the title [Lens] sharing logic for extracting icon type [Discover] sharing logic for extracting icon type Apr 6, 2022
@drewdaemon drewdaemon changed the title [Discover] sharing logic for extracting icon type [Discover] make field icons consistent across field list and doc tables Apr 6, 2022
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.

Thx so much! Now the icons is displayed correctly 👍 Added a request for refactoring to our utils folder for shared code, also saw some code as comment ready for a cleanup.

Note to myself: we currently have 2 ways in the code to get the label of the icon, version field's label in the DocViewer table is unknown therefore. We should also refactor this to a nice helper function in out utils folder to have a single point of 'failure', but that can be done in a follow up PR, there's another PR inflight working with field types , so there a good opportunity to simplify after that : #126657

@drewdaemon drewdaemon requested a review from kertal April 7, 2022 16:45
@drewdaemon
Copy link
Contributor Author

You got it @kertal ! Updated the PR with your requests.

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.

Thx so much Andrew, LGTM , tested locally and works as expected 👍

@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 445 446 +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 400.5KB 400.7KB +252.0B

History

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

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.2

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Discover Discover Application release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.2.0 v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover] <FieldIcon/> usage is inconsistent
5 participants