-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[ML] Migrate field-type-icon to EUI/React. #19159
Conversation
💚 Build Succeeded |
Pinging @elastic/ml-ui |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although a separate issue should be created to switch from the font awesome icons to EuiIcons.
let iconClass = ''; | ||
let iconChar = ''; | ||
|
||
switch (type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should migrate these Font Awesome icons to EUI icons I think. There are suitable replacements for most of these:
- fa-adjust - invert
- fa-clock-o - clock
- hash symbol - number
- fa-file-text-o - document (although would be nice to have one for a text style document)
I can't find any suitable replacements for
- fa-globe
- fa-laptop
You should raise an issue in the eui repo for the icons we need added.
💚 Build Succeeded |
Migrates the field-type-icon directive to use EUI/React. Instead of the angular/bootstrap tooltip attribute on the directive's tag, there's now an option tooltip-enabled which when true wraps the component inside EuiToolTip.
💔 Build Failed |
Migrates the
field-type-icon
directive to use EUI/React.Instead of the angular/bootstrap
tooltip
attribute on the directive's tag, there's now an optiontooltip-enabled
which whentrue
wraps the component insideEuiToolTip
.Part of #18374.