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.

Sign up for GitHub

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

Jump to bottom

[Vislib] Removes fontAwesome usage #113516

Merged
merged 5 commits into from
Oct 4, 2021
Merged

[Vislib] Removes fontAwesome usage #113516

merged 5 commits into from
Oct 4, 2021

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Sep 30, 2021 •
edited
Loading

Summary

Closes #104377

This PR removes the usage of the fontAwesome from the vislib plugin and with it it also removed the dependency from the kibanaLegacy plugin.

Specifically:

  1. It uses EuiIcon on the partial data instead of the fa-info icon

Screenshot 2021-09-30 at 11 14 07 AM

  1. We removed the alerts from vislib. Alerts were used to display an info callout about the labels (if there was no enough space to be displayed). This was broken for many minors so we feel quite comfortable to remove it as nobody has compalined about it. With this change we also remove the Display warnings option in the legacy gauge

image

@stratoula stratoula added release_note:breaking Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.16.0 v8.0.0 Feature:Vislib Vislib chart implementation backport:skip This commit does not require backporting and removed v7.16.0 labels Sep 30, 2021
@stratoula stratoula marked this pull request as ready for review September 30, 2021 14:44
@stratoula stratoula requested review from a team as code owners September 30, 2021 14:44
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors (Team:VisEditors)

mbondyra
mbondyra approved these changes Oct 1, 2021
View reviewed changes
Copy link
Contributor

@mbondyra mbondyra left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested on FF, works good. Code LGTM 🆗

for other reviewers: the warning can be displayed by creating date histogram aggregation with interval bigger than the time picker span
Screenshot 2021-10-01 at 10 29 22

cchaos
cchaos approved these changes Oct 1, 2021
View reviewed changes
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

🥇 Thanks @stratoula !!

src/plugins/vis_types/vislib/public/vislib/components/tooltip/_tooltip.scss Outdated Show resolved Hide resolved
@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
visTypeVislib 193 192 -1

Async chunks

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

id before after diff
visTypeVislib 378.2KB 373.7KB -4.5KB

History

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

@stratoula stratoula merged commit 7162f16 into elastic:master Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbondyra mbondyra mbondyra approved these changes

@cchaos cchaos cchaos approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Vislib Vislib chart implementation release_note:breaking Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Vislib] Remove font awesome usage
5 participants
@stratoula @elasticmachine @kibanamachine @cchaos @mbondyra