-
Notifications
You must be signed in to change notification settings - Fork 893
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 advanced_settings
plugin
#3963
Comments
Does that make them candidates for OUI? I'm still not 100% sure on OUI's stance on higher order components. Maybe @KrooshalUX can add some context here
Does that mark a gap in layouts that OUI provides? Or are the layouts achievable using OUI components? |
Great run down, thanks @SergeyMyssak ! HOCs need to be evaluated and treated in a case-by-case basis (vague, I know...) there really isn't a once-size-fits-all answer here, especially since a lot of context for implementation decisions has been lost over time. As a next step before I can make any recommendation from UX/OSDS would be to have engineering describe what functionality the higher order components introduce. For example - lets use the Search HOC - Looking at the comment the EUI component was broken at the time of build, so there was a "hack" introduced to implement incremental search. In later versions of the EuiSearchBar component, that issue has been fixed, therefore (potentially/hopefully) eliminating the need for the HOC. |
From my point of view, we have to choose carefully which components to move to the OUI and which ones are better left. It is not always advantageous to move components to the component library. I've summarized some of the points we should pay attention to:
Overview of components and their portability to the OUI:
@KrooshalUX, I have a question about the This component is only used in one place and for one purpose, to announce the entered query in the This component is unique and was put into a separate component to reduce the amount of code in the parent component. We can leave everything as it is. This component was discussed during office hours. In short, we have two components: field (advanced_settings) and field (saved_objects_management) where we duplicate the logic. Action items we need to discuss:
By making these functionalities, we may encounter problems number 1, 2, 3, and 4. We use this component to compose our fields and the bottom bar. In this component, we use Analysis of All the styles that are in the As for the
I don't see any visual change after removing this style, but I don't know where to find the I hope I have fully covered this plugin, if you still have questions I will be happy to answer |
@SergeyMyssak See #4329 for design guidance for Advanced Settings which may avoid the need for some of the recommendations here. |
Audit
I performed the OUI Usage audit to the
advanced_settings
plugin and got the following results.This plugin has 8 custom components:
management_app/components
: advanced_settings_voice_announcement, call_outs, field, form, searchcomponent_registry
: PageTitle, PageSubtitle, PageFooterThe image below shows all the components listed except
advanced_settings_voice_announcement
,PageSubtitle
andPageFooter
, where the text in red is the name of the component.This plugin also has one
scss
file - _advanced_settings.scss. The image below shows the use of the classnames written in the file (except .osdBody--mgtAdvancedSettingsHasBottomBar .mgtPage__body), where the text in red is the classname.Conclusion
PageSubtitle
andPageFooter
are functions that returnnull
and can be removed if they are not to be used in the futurescss
styles are used for additional placement or to add external effects that do not change the appearance of the components in the OUI.mgtAdvancedSettingsForm__button
classname is unnecessary and does not affect anything, so it can be removedOpenSearch-Dashboards/src/plugins/advanced_settings/public/management_app/_advanced_settings.scss
Lines 76 to 78 in a8ace28
The text was updated successfully, but these errors were encountered: