[Research] OUI Compliance audit of the inspector plugin

[BSFishy]

Tracking issue for OUI compliance audit of the inspector plugin.

Follow the steps in #3945 to complete the audit, then open another issue with findings. Once the work of performing the audit is done, this issue can be closed.

Related files

size (bytes)	file	notes
262	src/plugins/inspector/public/views/requests/_requests.scss
230	src/plugins/inspector/public/ui/inspector_panel.scss
164	src/plugins/inspector/public/views/data/_data_table.scss
52	src/plugins/inspector/public/views/_index.scss
24	src/plugins/inspector/public/views/data/_index.scss
23	src/plugins/inspector/public/index.scss
22	src/plugins/inspector/public/views/requests/_index.scss

If this issue isn't already assigned to someone and you'd like to take on this work, please ping @BSFishy in the comments and I can assign it to you.

[nurSaadat]

src/plugins/inspector/public/views/_index.scss imports styles from src/plugins/inspector/public/views/data/_index.scss and
src/plugins/inspector/public/views/requests/_index.scss.

src/plugins/inspector/public/views/data/_index.scss imports styles from src/plugins/inspector/public/views/data/_data_table.scss.

src/plugins/inspector/public/index.scss imports styles from src/plugins/inspector/public/views/_index.scss.

src/plugins/inspector/public/views/requests/_index.scss imports styles from src/plugins/inspector/public/views/requests/_requests.scss.

As a dependency graph, it looks like this:

  graph TD;
      public/index.scss-->public/views/_index.scss;
      public/views/_index.scss-->public/views/data/_index.scss;
      public/views/_index.scss-->public/views/requests/_index.scss;
      public/views/data/_index.scss-->public/views/data/_data_table.scss;
      public/views/requests/_index.scss-->public/views/requests/_requests.scss;

Loading

Therefore, the main files to look at are _requests.scss, inspector_panel.scss and _data_table.scss.

data.table.scss

_data_table.scss has only two custom classes insDataTableFormat__filter and insDataTableFormat__table. They are used to change the visibility of a EuiButtonIcon by changing its opacity on focus or on hover of a table row

OpenSearch-Dashboards/src/plugins/inspector/public/views/data/_data_table.scss

Lines 1 to 8 in 20c928f

	.insDataTableFormat__filter {
	opacity: 0;
	}
	.insDataTableFormat__filter:focus,
	.insDataTableFormat__table tr:hover .insDataTableFormat__filter {
	opacity: 1;
	}

I think, those properties are still needed, as OUI doesn't currently allow this type of behaviour for a button icon.

Example can be seen here

data_table.tsx relies mostly on EUI components. The only component which is not from '@elastic/eui' is FormattedMessage.

requests.scss

_requests.scss contains three custom classes, all of them use euiSize variables to define properties.

OpenSearch-Dashboards/src/plugins/inspector/public/views/requests/_requests.scss

Lines 1 to 14 in 20c928f

	.insRequestDetailsStats__icon {
	margin-right: $euiSizeS;
	}
	.insRequestSelector__singleRequest {
	height: $euiButtonHeightSmall;
	padding: 0 $euiSizeS;
	display: flex;
	align-items: center;
	}
	.insRequestSelector__menuSpinner {
	margin-left: $euiSizeS;
	}

In request_selector.tsx file a div is styled using insRequestSelector__singleRequest custom class. That div is used for the logical expression and already wrapped by EuiFlexItem. I think it may be right to leave it as it is.

OpenSearch-Dashboards/src/plugins/inspector/public/views/requests/components/request_selector.tsx

Lines 163 to 173 in 20c928f

	<EuiFlexItem grow={true}>
	{requests.length <= 1 && (
	<div
	className="insRequestSelector__singleRequest"
	data-test-subj="inspectorRequestName"
	>
	{selectedRequest.name}
	</div>
	)}
	{requests.length > 1 && this.renderRequestDropdown()}
	</EuiFlexItem>

inspector_panel.scss

inspector_panel.scss has a single class insInspectorPanel__flyoutBody and a comment

OpenSearch-Dashboards/src/plugins/inspector/public/ui/inspector_panel.scss

Lines 1 to 12 in 20c928f

	.insInspectorPanel__flyoutBody {
	// TODO: EUI to allow for custom classNames to inner elements
	// Or supply this as default
	> div {
	display: flex;
	flex-direction: column;
	> div {
	flex-grow: 1;
	}
	}
	}

I manually tested if something is changing when I delete that class. I suppose now it is redundant, since Flyout component behaved exactly the same way without a custom class.

Conclusion:

• OUI components and variables mostly are used throughout the plugin
• No need to remove custom class for EuiButtonIcon
• Remove insInspectorPanel__flyoutBody custom class

[nurSaadat]

@BSFishy could you please assign me this issue?

[joshuarrrr]

@nurSaadat When you get a chance, can you just copy/paste your audit to a separate issue?