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

Remove usages of @kbn/ui-framework #46410

Closed
kobelb opened this issue Sep 24, 2019 · 35 comments · Fixed by #51696 or #167833
Closed

Remove usages of @kbn/ui-framework #46410

kobelb opened this issue Sep 24, 2019 · 35 comments · Fixed by #51696 or #167833
Assignees
@Ikuni17 @vadimkibana
Labels
chore Team:Operations Team label for Operations Team Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) technical debt Improvement of the software architecture and operational architecture

Comments

@kobelb
Copy link
Contributor

kobelb commented Sep 24, 2019

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.
@kobelb kobelb added chore Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 24, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@kobelb
Copy link
Contributor Author

kobelb commented Nov 15, 2019

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

@mbondyra mbondyra mentioned this issue Nov 26, 2019
Merged
7 tasks
@tylersmalley tylersmalley reopened this Sep 23, 2020
@tylersmalley
Copy link
Contributor

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 #78230, but quickly realized I am not familiar enough with EUI nor Discover and Timelion to take on with the time I have.

@tylersmalley
Copy link
Contributor

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

@tylersmalley
Copy link
Contributor

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

@cchaos
Copy link
Contributor

cchaos commented Sep 23, 2020

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
Copy link
Contributor

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

@kertal
Copy link
Member

kertal commented Sep 24, 2020

@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 tylersmalley mentioned this issue Sep 24, 2020
Merged
@tylersmalley
Copy link
Contributor

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

This was referenced Oct 8, 2020
Closed
Closed
@timroes timroes added the technical debt Improvement of the software architecture and operational architecture label Nov 4, 2020
@timroes timroes mentioned this issue Nov 5, 2020
Closed
@mbondyra mbondyra removed their assignment Nov 16, 2020
This was referenced Dec 8, 2020
Closed
Merged
@flash1293
Copy link
Contributor

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.

@flash1293 flash1293 removed their assignment Mar 2, 2021
@kertal
Copy link
Member

kertal commented Mar 2, 2021

Removing it in Discover is on our todo list:
#92573

@stratoula stratoula removed the Team:Visualizations Visualization editors, elastic-charts and infrastructure label May 30, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label May 30, 2023
@stratoula
Copy link
Contributor

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

@dej611
Copy link
Contributor

dej611 commented Jun 15, 2023

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
Copy link
Contributor

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

@MichaelMarcialis
Copy link
Contributor

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?

  • Folder open = folderOpen
  • Cube = kubernetesPod
  • Key
  • Bank
  • Automobile
  • Home = home
  • Question = questionInCircle
  • Plane
  • File open = document
  • User = user
  • Users = users
  • Music
  • Flag = flag
  • Tag = tag
  • Phone
  • Desktop = desktop
  • Font = lettering
  • At
  • Heart = heart
  • Bolt = bolt
  • Map marker = mapMarker
  • 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.

image

CCing @elastic/eui-team.

@nreese
Copy link
Contributor

nreese commented Jun 20, 2023

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
Copy link
Contributor

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

@legrego legrego mentioned this issue Jun 21, 2023
Closed
@legrego
Copy link
Member

legrego commented Jun 21, 2023

AppEx Platform Security is tracking our work here: #160122

@stratoula
Copy link
Contributor

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

@nreese
Copy link
Contributor

nreese commented Jun 21, 2023

@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
Copy link
Contributor

@nreese this is a good idea!

@dej611
Copy link
Contributor

dej611 commented Jun 21, 2023

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

@stratoula
Copy link
Contributor

Thanx otherwise, we will need to create them on EUI

@stratoula stratoula mentioned this issue Jun 22, 2023
Closed
38 tasks
@stratoula
Copy link
Contributor

I created a new issue for the graph #160232 so we are going to track our work there.

@legrego
Copy link
Member

legrego commented Sep 11, 2023

AppEx Platform Security is tracking our work here: #160122

This has merged

@stratoula stratoula removed the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Sep 12, 2023
@stratoula
Copy link
Contributor

We removed it from graph!

@Ikuni17
Copy link
Contributor

Ikuni17 commented Sep 14, 2023 •
edited
Loading

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

@stratoula stratoula mentioned this issue Sep 18, 2023
Closed
@stratoula
Copy link
Contributor

stratoula commented Sep 18, 2023 •
edited
Loading

aaa I forgot the classnames. I created an issue here #166583. This is easy, we will take care of it

Update: Done! #166588

@vadimkibana vadimkibana added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Sep 22, 2023
@vadimkibana vadimkibana self-assigned this Sep 22, 2023
@vadimkibana vadimkibana mentioned this issue Sep 22, 2023
Merged
@vadimkibana
Copy link
Contributor

vadimkibana commented Sep 22, 2023 •
edited
Loading

@elastic/appex-sharedux part should be done here #167014

@ThomThomson ThomThomson mentioned this issue Sep 22, 2023
Merged
@ThomThomson
Copy link
Contributor

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.

vadimkibana added a commit that referenced this issue Sep 27, 2023
@vadimkibana @kibanamachine
use EUI components for tutorial params (#167014)
94bd19d 
## Summary

Partially addresses #46410

Stops using `kuiTextInput` CSS class name in `number_parameter.js` and
`string_paramter.js` components in the `home` plugin.

How to test? I don't know if these parameters are still used, so to test
this apply this patch:

```diff
diff --git a/src/plugins/home/public/application/components/tutorial/instruction_set.js b/src/plugins/home/public/application/components/tutorial/instruction_set.js
index 651212f062c..7f2077a322d 100644
--- a/src/plugins/home/public/application/components/tutorial/instruction_set.js
+++ b/src/plugins/home/public/application/components/tutorial/instruction_set.js
@@ -261,14 +261,20 @@ class InstructionSetUi extends React.Component {
 
   render() {
     let paramsForm;
-    if (this.props.params && this.state.isParamFormVisible) {
+    if (true) {
       paramsForm = (
         <>
+          PARAMETER FORM
           <EuiSpacer />
           <ParameterForm
-            params={this.props.params}
-            paramValues={this.props.paramValues}
-            setParameter={this.props.setParameter}
+            params={[
+              { id: 'test', label: 'test', type: 'string' },
+              { id: 'test2', label: 'test2', type: 'number'}
+            ]}
+            paramValues={{ test: 'test', test2: 123 }}
+            setParameter={(id, value) => {
+              console.log('setParameter', id, value);
+            }}
           />
         </>
       );
```

And go to `/app/home#/tutorial/apm` page, you will see the new parameter
input look there:

<img width="478" alt="image"
src="https://github.com/elastic/kibana/assets/82822460/ba3a3dce-c2d5-41db-a7e8-24bf6caeb16b">

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@Ikuni17 Ikuni17 mentioned this issue Oct 2, 2023
Merged
Ikuni17 added a commit that referenced this issue Oct 4, 2023
@Ikuni17 @kibanamachine
Remove KUI Framework (#167833)
2ff938e 
## Summary

Closes #46410

Removes the deprecated `@kbn/ui-framework`

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Ikuni17 Ikuni17

@vadimkibana vadimkibana

Labels
chore Team:Operations Team label for Operations Team Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove usages of @kbn-ui-framework mbondyra/kibana
Remove KUI Framework Ikuni17/kibana