-
Notifications
You must be signed in to change notification settings - Fork 919
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
[Research] [CCI] OUI Usage Audit in console
plugin
#3966
Comments
opensearch-project/oui#776 (for tracking :) )
Do you want to create a feature proposal for this in OUI? If not, I can |
@BSFishy Yes, I created a feature proposal opensearch-project/oui#783 |
@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? |
At a high level, it may also be worth analyzing this component in the context of #4160, because
Yeah, that's one valid way to go, but I'd wait on #4160, first.
Yeah, let's remove.
Probably
Agree with this approach. Can you also leave a comment linking to that finding on #4089? It will help whoever picks that up start with the context.
These are likely antipatterns we can get updated design guidance on once we provide screenshots.
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). |
@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 |
@joshuarrrr Thank for the feedback! I will take an incremental approach and do it one by one starting with |
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
#consoleRoot
,.conApp
,.conApp__editor
,.conApp__output
,.conApp__editorContent
,.conApp__outputContent
These are styling for the <div> containers with flex that should be replaceable by OUI flex components..conApp__autoComplete
Is a classname that used by single <ul> element that looks like is no longer used. The same thing was mentioned in comments.OpenSearch-Dashboards/src/plugins/console/public/styles/_app.scss
Lines 80 to 90 in a8ace28
.conApp__editorActions
,.conApp__editorActionButton
,.conApp__editorActionButton--success
Used for styling of action buttons. The <button> was used insted of OUI components. If replaced less styling will likely be used..conApp__resizer
This is classname for the resizer element (implemented in <button> tag) that dynamically changes size of editor and output fields. Not likely to be removable, unless the separate OUI component will be implement later..conApp__settingsModal
Setsmin-width: 460px
for the EuiModal. This component does accept prop formax-width
, but there is no prop formin-width
. Might be reasonable to add it to the OuiModal implementation..conApp__requestProgressBarContainer
position styling for the progress bar..conApp__tabsExtension
setsborder-bottom: $euiBorderThin;
_history.scss
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._help.scss
.cconHelp__example
used for styling example code block <div> container.After analyzing css styles, I investigated usage of native html tags.
console/application/containers/editor/legacy/editor.tsx
We could possibly replace them with OUI container or list componentsConclusion
A large chunk of the custom styling are aimed to style native html elements, especially containers that use flex. So converting them to OUI will reduce number of custom stylings. Furthermore most if not all native html elements can be converted to OUI, with exception of places where the raw html is expected by the OUI components.
Files with native html elements in
console/public/application/
(except when used appropriately with OUI):./components/
: console_menu, editor_content_spinner, editor_example./containers/console_history
: console_history, history_viewer./containers/main
: main./containers/editor/legacy/console_editor
: editor_output, editor./containers/editor
: editorSuggestions
Based on my finding I have a few suggestions.
flex: x y z;
is often used, so adding support for it might be worth it.OuiResizableContainer
, but we don't utilize it here and instead custom componentPanelsContainer
fromopensearch_dashboards_react/public
was used. I believe we should switch from it to OUI component.OpenSearch-Dashboards/src/plugins/console/public/application/containers/editor/editor.tsx
Lines 92 to 109 in ca0bb8f
minWidth
option for the OuiModal component. Currently it only gotmaxWidth
prop, so we have to use custom styling to setmin-width
css property.Action Plan
The text was updated successfully, but these errors were encountered: