Skip to content

[Discover] make field icons consistent across field list and doc tables#129621

Merged
drewdaemon merged 5 commits intoelastic:mainfrom
drewdaemon:124017/use-field-icon-consistently-in-discover
Apr 11, 2022
Merged

[Discover] make field icons consistent across field list and doc tables#129621
drewdaemon merged 5 commits intoelastic:mainfrom
drewdaemon:124017/use-field-icon-consistently-in-discover

Conversation

@drewdaemon
Copy link
Copy Markdown
Contributor

@drewdaemon drewdaemon commented Apr 6, 2022 •
edited
Loading

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 (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// 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
Copy Markdown
Contributor

elasticmachine commented Apr 6, 2022

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
kertal
kertal reviewed Apr 7, 2022
View reviewed changes
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.

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
Copy Markdown
Contributor Author

drewdaemon commented Apr 7, 2022

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

kertal
kertal approved these changes Apr 7, 2022
View reviewed changes
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.

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

@drewdaemon
Copy link
Copy Markdown
Contributor Author

drewdaemon commented Apr 11, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Apr 11, 2022

💚 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

@drewdaemon drewdaemon merged commit 6954b0f into elastic:main Apr 11, 2022
@kibanamachine kibanamachine mentioned this pull request Apr 11, 2022
Merged
kibanamachine pushed a commit that referenced this pull request Apr 11, 2022
@drewdaemon
[Discover] make field icons consistent across field list and doc tabl…
5921457 
…es (#129621)

(cherry picked from commit 6954b0f)
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Apr 11, 2022

💚 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
@kibanamachine @drewdaemon
[Discover] make field icons consistent across field list and doc tabl…
eefebd4 
…es (#129621) (#129935)

(cherry picked from commit 6954b0f)

Co-authored-by: Andrew Tate <andrew.tate@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kertal kertal kertal approved these changes

Assignees

No one assigned

Labels

auto-backport Deprecated - use backport:version if exact versions are needed Feature:Discover Discover Application release_note:fix Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// v8.2.0 v8.3.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Discover] <FieldIcon/> usage is inconsistent

5 participants

@drewdaemon @elasticmachine @kibana-ci @kibanamachine @kertal