opensearch-project / OpenSearch-Dashboards

📊 Open source visualization dashboards for OpenSearch.
https://opensearch.org/docs/latest/dashboards/index/
Apache License 2.0
1.69k stars 891 forks source link

[Research] [CCI] OUI Usage Audit in ```console``` plugin #3966

Open curq opened 1 year ago

curq commented 1 year ago

I have preformed audit of the console plugin. To start I identified all .scss files and found that custom styling is used in 3 of them (the rest are files with only imports). They are _app.scss, _history.scss and _help.scss. To start, I decided to look through every custom style and investigate what they do.


_app.scss

BSFishy commented 1 year ago

Currently OuiFlexItem only have props for flex grow, and not for shrink and basis. While looking through custom styles I noticed that flex: x y z; is often used, so adding support for it might be worth it.

https://github.com/opensearch-project/oui/issues/776 (for tracking :) )

Add minWidth option for the OuiModal component. Currently it only got maxWidth prop, so we have to use custom styling to set min-width css property.

Do you want to create a feature proposal for this in OUI? If not, I can

curq commented 1 year ago

@BSFishy Yes, I created a feature proposal https://github.com/opensearch-project/oui/issues/783

joshuarrrr commented 1 year ago

@curq Can you add some screenshots to this audit so that it's easier to understand visually and functionally what some of these components and styles are doing?

joshuarrrr commented 1 year ago

At a high level, it may also be worth analyzing this component in the context of https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4160, because dev_tools is the wrapper for this functionality, and fixing it's layout to utilize https://oui.opensearch.org/1.0/#/layout/page may make many of the layout styles here unnecessary. I've also provided some detailed feedback:

consoleRoot, .conApp, .conAppeditor, .conAppoutput, .conAppeditorContent, .conAppoutputContent These are styling for the
containers with flex that should be replaceable by OUI flex components.

Yeah, that's one valid way to go, but I'd wait on https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4160, first.

.conApp__autoComplete Is a classname that used by single

    element that looks like is no longer used. The same thing was mentioned in comments.

Yeah, let's remove.

.conAppeditorActions, .conAppeditorActionButton, .conApp__editorActionButton--success Used for styling of action buttons. The

Probably OuiButtonIcon for the action buttons.

.conApp__resizer This is classname for the resizer element (implemented in

Agree with this approach. Can you also leave a comment linking to that finding on https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4089? It will help whoever picks that up start with the context.

.conApprequestProgressBarContainer position styling for the progress bar. .conApptabsExtension sets border-bottom: $euiBorderThin;

These are likely antipatterns we can get updated design guidance on once we provide screenshots.

All stylings are either minor adjustments like overflow: auto;, color: $euiColorMediumShade; or styling for the html elements used as flex container. Some of them can be removed by using OUI components instead.

We should aim to remove all these styles. Start by moving to OUI components, and then we can see what styles remain.(For example, overflow can be handled by https://oui.opensearch.org/1.0/#/utilities/css-utility-classes, but it's probably not needed at all).

joshuarrrr commented 1 year ago

@curq It sounds like you have a good plan. The next step is to create issues for actually implementing these changes. The first issue could either be to use OuiResizableContainer, or else to start by replacing all the low-level native components with OUI equivalents. You can make multiple issues at once, or you can do it incrementally, making one issue and a PR to solve that problem, and then moving on to the next phase.

curq commented 1 year ago

@joshuarrrr Thank for the feedback! I will take an incremental approach and do it one by one starting with OuiResizableContainer. During working on this first issue I will make sure to capture screenshots for better visual overview. I will update this audit after that.