[Research] OUI Compliance audit of the `saved_objects` plugin

[sayuree]

Audit

The saved_objects plugin contains 3 Sass files: index.scss, saved_object_save_modal.scss and _index.scss. index.scss and _index.scss are importing a style from saved_object_save_modal.scss, which has a single class: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.scss#L1-L3 https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/_index.scss#L1

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/index.scss#L1

Related React code: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx#L102-L106

There are no img, span, but div, p HTML tags are present, outnumbered by OUI library components. When a specific value was needed, OUI variable was used: euiSizeXXL The custom components are predominantly made of OUI library components and are used correctly: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.test.tsx#L40-L47

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.test.tsx#L54-L62

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx#L102-L166

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx#L265-L274

However, HTML tags are also present in the following places: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx#L284-L310 For the focus management, we have ref and tabindex defined in div tag. I assume we could use EuiPanel with the property panelRef of type Ref<HTMLDivElement> and extends HTMLElement, allowing us to define tabindex property as well. https://github.com/opensearch-project/OpenSearch-Dashboards/blob/bd7d70731eb56372aa2a360e85207a1e6396091f/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx#L320-L334 FormattedMessage is wrapped into p tag for customized rendering (recommended practice):

By default will render the formatted string into a . If you need to customize rendering, you can either wrap it with another React element (recommended), specify a different tagName (e.g., 'div'), or pass a function as the child.

Conclusion

1. OUI library components are mainly used throughout the saved_objects plugin, there are no issues with the usage;
2. Replace tags used for wrapping other components with EuiPanel, including cases related to focus management;
3. No need to remove p tag, enveloping FormattedMessage;

[joshuarrrr]

@sayuree Thanks for the audit, there's lots of useful analysis here.

...importing a style from saved_object_save_modal.scss, which has a single class:

The first goal of these audits is to simply identify the styles and components, as you've done here. However, when analyzing and recommending mitigations, the goal is to fully remove any custom styles, such as this one. To do so, we need to try to figure out 1) what the style is doing, and 2) how we might solve the same problem, or achieve similar outcomes with a more compliant approach.

In this case, 1) is fairly obvious: it's setting a static width of the modal. But why? The most likely reasons are either to specify a different minimum or maximum modal width. Each of those scenarios is already accounted for in OUI, without the need for a custom class and style: https://oui.opensearch.org/1.0/#/layout/modal

To help UX assess whether a custom width is necessary at all, can you provide a screenshot of this modal and instructions on how to navigate/activate it on https://playground.opensearch.org ?