Remove usages of `@kbn/ui-framework`

kobelb commented 5 years ago

The @kbn/ui-framework is still used in a few places, and references an incredibly old version of EUI. The version of EUI has a reference to a version of lodash which is flagged by security vulnerability scanners. Even though the existing usages are a false positive, it causes a lot of headaches.

elasticmachine commented 5 years ago

Pinging @elastic/kibana-app

kobelb commented 5 years ago

Any volunteers to address this? The usages are limited to https://github.com/elastic/kibana/blob/dbc9a52fd84fcf0f293018975b665b2db6d28451/src/legacy/core_plugins/kibana/public/visualize/listing/no_visualizations_prompt.js and https://github.com/elastic/kibana/blob/75f31130f0392387e2f791bd8f143707d8055b7a/src/plugins/kibana_react/public/exit_full_screen_button/exit_full_screen_button.tsx

tylersmalley commented 4 years ago

I opened up a PR to remove the @kbn/ui-framework package and noticed we're still referencing the Kui* styles.

We will want to replace these usages with the equivalent EUI styles. I started down this road in https://github.com/elastic/kibana/pull/78230, but quickly realized I am not familiar enough with EUI nor Discover and Timelion to take on with the time I have.

tylersmalley commented 4 years ago

Anyone have some spare cycles so we can push @kbn/ui-framework out to sea and wish it farewell?

tylersmalley commented 4 years ago

cc @elastic/kibana-design, not sure if this is something you all want to be in the loop on.

cchaos commented 4 years ago

Kibana designers would too love to wish KUI farewell! 😁 We can try to help if there are specific instances that are questionable about replacements. I'm pretty sure there's already a Discover feature branch that has converted this all to EUI (cc @kertal). Timelion is the harder one because I think it might still have Angular bits and bootstrap.

tylersmalley commented 4 years ago

While it is in Angular - from what I can tell it's all in HTML templates, so should be doable.

kertal commented 4 years ago

@cchaos there's not much kui left in discover legacy table & discover context. if it should be done fast, we could inline the needed kui directives, this could also be done with timelion. context.html is next to be deangularized, so kui should be gone there, too

tylersmalley commented 4 years ago

@kertal I am 👍 on inlining for what it's worth.

flash1293 commented 3 years ago

I can see references to kui in:

discover
home
kibana_legacy (used by timelion and discover)
maps_legacy
graph (heavy usage)
ml

I'm not planning to work on this right now, so I'm going to remove my assignment.

kertal commented 3 years ago

Removing it in Discover is on our todo list: https://github.com/elastic/kibana/issues/92573

stratoula commented 1 year ago

Removing visualizations team as we don't ise this plugin anymore!

kertal commented 1 year ago

Looks like this code can be removed? https://github.com/elastic/kibana/blob/1b8581540295fde746dae6b4a09d74fb5821bfef/src/dev/build/tasks/package_json/find_used_dependencies.ts#L40 don't see any '@kbn/ui-framework' usage anymore?

elasticmachine commented 1 year ago

Pinging @elastic/kibana-operations (Team:Operations)

Ikuni17 commented 1 year ago

We still have the following references to kui* classes, which seem like they need addressed before full removal. If you'd like to replicate this search the regex is: className\s*=\s*["{](kui[^"}]*)?["}]. The codeowners are:

src/plugins/home/public/* @elastic/appex-sharedux x-pack/plugins/graph/* @elastic/kibana-visualizations

src/plugins/home/public/application/components/tutorial/number_parameter.js
  24,11:           className="kuiTextInput"

src/plugins/home/public/application/components/tutorial/string_parameter.js
  21,16:         <input className="kuiTextInput" type="text" value={value} onChange={handleChange} />

x-pack/plugins/graph/public/components/control_panel/control_panel_tool_bar.tsx
  94,13:             className="kuiButton kuiButton--basic kuiButton--small"
  100,19:             <span className="kuiIcon fa-history" />
  108,13:             className="kuiButton kuiButton--basic kuiButton--small"
  114,19:             <span className="kuiIcon fa-repeat" />
  122,13:             className="kuiButton kuiButton--basic kuiButton--small"
  127,19:             <span className="kuiIcon fa-plus" />
  135,13:             className="kuiButton kuiButton--basic kuiButton--small"
  140,19:             <span className="kuiIcon fa-link" />
  149,13:             className="kuiButton kuiButton--basic kuiButton--small"
  154,19:             <span className="kuiIcon fa-trash" />
  162,13:             className="kuiButton kuiButton--basic kuiButton--small"
  167,19:             <span className="kuiIcon fa-ban" />
  175,13:             className="kuiButton kuiButton--basic kuiButton--small"
  180,19:             <span className="kuiIcon fa-paint-brush" />
  188,13:             className="kuiButton kuiButton--basic kuiButton--small"
  193,19:             <span className="kuiIcon fa-info" />
  203,15:               className="kuiButton kuiButton--basic kuiButton--small"
  208,21:               <span className="kuiIcon fa-play" />
  219,15:               className="kuiButton kuiButton--basic kuiButton--small"
  223,21:               <span className="kuiIcon fa-pause" />

x-pack/plugins/graph/public/components/control_panel/drill_down_icon_links.tsx
  38,13:             className="kuiButton kuiButton--basic kuiButton--small"

x-pack/plugins/graph/public/components/control_panel/drill_downs.tsx
  21,15:         <span className="kuiIcon fa-info" />
  44,27:                     <span className="kuiIcon gphNoUserSelect">{urlTemplate.icon?.code}</span>{' '}

x-pack/plugins/graph/public/components/control_panel/merge_candidates.tsx
  47,15:         <span className="kuiIcon fa-link" />
  108,19:                   className="kuiButton kuiButton--basic kuiButton--small"
  111,25:                   <span className="kuiIcon fa-chevron-circle-right" />
  121,19:                   className="kuiButton kuiButton--basic kuiButton--small"
  125,25:                   <span className="kuiIcon fa-chevron-circle-left" />

x-pack/plugins/graph/public/components/control_panel/select_style.tsx
  21,15:         <span className="kuiIcon fa-paint-brush" />
  38,15:               className="kuiIcon gphColorPicker__color fa-circle"

x-pack/plugins/graph/public/components/control_panel/selected_node_editor.tsx
  51,13:             className="kuiButton kuiButton--basic kuiButton--iconText kuiButton--small"
  54,19:             <span className="kuiButton__icon kuiIcon fa-object-group" />
  63,13:             className="kuiButton kuiButton--basic kuiButton--iconText kuiButton--small"
  66,19:             <span className="kuiIcon fa-object-ungroup" />

x-pack/plugins/graph/public/components/control_panel/selection_tool_bar.tsx
  80,13:             className="kuiButton kuiButton--basic kuiButton--small"
  94,13:             className="kuiButton kuiButton--basic kuiButton--small"
  109,13:             className="kuiButton kuiButton--basic kuiButton--small"
  123,13:             className="kuiButton kuiButton--basic kuiButton--small"

x-pack/plugins/graph/public/components/settings/blocklist_form.tsx
  44,41:               values={{ stopSign: <span className="kuiIcon fa-ban" /> }}

Additionally, there are a few direct imports of the CSS files I'm not 100% sure about. I don't think they're doing anything, but I'd like to be sure.

@elastic/kibana-presentation https://github.com/elastic/kibana/blob/main/x-pack/plugins/canvas/shareable_runtime/index.ts#L12

@elastic/kibana-security https://github.com/elastic/kibana/blob/main/x-pack/plugins/security/server/prompt_page.tsx#L91

elasticmachine commented 1 year ago

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

stratoula commented 1 year ago

I forgot about Graph :/ This will be tricky as there are no eui icons that can be a match for all the choices we give to the users.

dej611 commented 1 year ago

Maybe we could request EUI to provide the same icon set used in Graph so that we can remove FontAwesome once and for all?

stratoula commented 1 year ago

Def something we need to discuss with them! @elastic/kibana-design wdyt about it?

MichaelMarcialis commented 1 year ago

The Graph app doesn't appear to offer an excessive number of icons. Even less that don't have an EUI equivalent (I count 8). I imagine we could add icons for the unmatched ones fairly easily. If that's how ya'll would like to proceed, what sort of priority should this be given?

[x] Folder open = folderOpen
[x] Cube = kubernetesPod
[ ] Key
[ ] Bank
[ ] Automobile
[x] Home = home
[x] Question = questionInCircle
[ ] Plane
[x] File open = document
[x] User = user
[x] Users = users
[ ] Music
[x] Flag = flag
[x] Tag = tag
[ ] Phone
[x] Desktop = desktop
[x] Font = lettering
[ ] At
[x] Heart = heart
[x] Bolt = bolt
[x] Map marker = mapMarker
[x] Exclamation = warning
[ ] Industry

While we're on the topic of FontAwesome, is that what the Maps app uses for their icons as well? If so, would those icons need to be accounted for as well? If so, that may be more problematic, as the Maps app offers a much larger list of icon options.

CCing @elastic/eui-team.

nreese commented 1 year ago

While we're on the topic of FontAwesome, is that what the Maps app uses for their icons as well? If so, would those icons need to be accounted for as well? If so, that may be more problematic, as the Maps app offers a much larger list of icon options.

Maps gets its icons from https://github.com/elastic/maki

stratoula commented 1 year ago

Thanx @MichaelMarcialis for the list! About the priority let's discuss it today on our WG sync

legrego commented 1 year ago

AppEx Platform Security is tracking our work here: https://github.com/elastic/kibana/issues/160122

stratoula commented 1 year ago

@dej611 we suggest using maki icons that the maps team already uses. Wdyt?

nreese commented 1 year ago

@dej611 we suggest using maki icons that the maps team already uses. Wdyt?

We could move maki utilities into a package so its accessible anywhere in kibana

stratoula commented 1 year ago

@nreese this is a good idea!

dej611 commented 1 year ago

Need to check how the maki icons work. Will update the issue here once verified 👍

stratoula commented 1 year ago

Thanx otherwise, we will need to create them on EUI

stratoula commented 1 year ago

I created a new issue for the graph https://github.com/elastic/kibana/issues/160232 so we are going to track our work there.

legrego commented 1 year ago

AppEx Platform Security is tracking our work here: #160122

This has merged

stratoula commented 1 year ago

We removed it from graph!

Ikuni17 commented 1 year ago

Thanks everyone for the work so far. We're close! These are the remaining usages of KUI:

@stratoula
- [x] Multiple kuibutton x-pack/plugins/graph/public/components/control_panel/selection_tool_bar.tsx
@elastic/appex-sharedux
- [x] plugins/home/public/application/components/tutorial/number_parameter.js
- [x] plugins/home/public/application/components/tutorial/string_parameter.js
@elastic/kibana-presentation
- [x] x-pack/plugins/canvas/shareable_runtime/index.ts

stratoula commented 1 year ago

aaa I forgot the classnames. I created an issue here https://github.com/elastic/kibana/issues/166583. This is easy, we will take care of it

Update: Done! https://github.com/elastic/kibana/pull/166588

vadimkibana commented 1 year ago

@elastic/appex-sharedux part should be done here https://github.com/elastic/kibana/pull/167014

ThomThomson commented 1 year ago

The @elastic/kibana-presentation part will be done here. There were no usages of any kui classes, so I've just removed the stylesheet import entirely.

elastic / kibana

Remove usages of `@kbn/ui-framework` #46410