[Research] OUI Compliance Audit of dev_tools plugin

[sayuree]

Audit of #4078

The dev_tools plugin has 1 Sass file: index.scss:

OpenSearch-Dashboards/src/plugins/dev_tools/public/index.scss

Lines 8 to 34 in bd7d707

	.devApp__container,
	.devApp {
	flex: 1 1 auto;
	display: flex;
	flex-direction: column;
	> * {
	flex-shrink: 0;
	}
	}
	.devAppWrapper {
	display: flex;
	flex-direction: column;
	flex-grow: 1;
	}
	.devAppDataSourcePicker {
	margin: 7px 8px 0 0;
	min-width: 400px;
	}
	.devAppTabs {
	display: flex;
	flex-flow: row wrap;
	justify-content: space-between;
	}

1. The plugin does not contain img, span HTML tags, but uses main, div tags.
   

   OpenSearch-Dashboards/src/plugins/dev_tools/public/application.tsx

   Lines 156 to 209 in bd7d707

   	<main className="devApp">
   	<EuiTabs className="devAppTabs">
   	{devTools.map((currentDevTool) => (
   	<EuiToolTip content={currentDevTool.tooltipContent} key={currentDevTool.id}>
   	<EuiTab
   	disabled={currentDevTool.isDisabled()}
   	isSelected={currentDevTool === activeDevTool}
   	onClick={() => {
   	if (!currentDevTool.isDisabled()) {
   	updateRoute(`/${currentDevTool.id}`);
   	}
   	}}
   	>
   	{currentDevTool.title}
   	</EuiTab>
   	</EuiToolTip>
   	))}
   	{dataSourceEnabled ? (
   	<div className="devAppDataSourcePicker">
   	<EuiComboBox
   	aria-label={i18n.translate('devTool.devToolWrapper.DataSourceComboBoxAriaLabel', {
   	defaultMessage: 'Select a Data Source',
   	})}
   	placeholder={i18n.translate('devTool.devToolWrapper.DataSourceComboBoxPlaceholder', {
   	defaultMessage: 'Select a Data Source',
   	})}
   	singleSelection={{ asPlainText: true }}
   	options={dataSources}
   	selectedOptions={selectedOptions}
   	onChange={onChange}
   	prepend="DataSource"
   	compressed
   	isDisabled={!dataSourceEnabled}
   	/>
   	</div>
   	) : null}
   	</EuiTabs>
   	<div
   	className="devApp__container"
   	role="tabpanel"
   	data-test-subj={activeDevTool.id}
   	ref={async (element) => {
   	if (
   	element &&
   	(mountedTool.current === null ||
   	mountedTool.current.devTool !== activeDevTool ||
   	mountedTool.current.mountpoint !== element)
   	) {
   	await remount(element);
   	}
   	}}
   	/>
   	</main>

2. The plugin has a component DevToolsWrapper, which mostly uses OUI library components and occasionally uses HTML tags for wrapping:
   

   OpenSearch-Dashboards/src/plugins/dev_tools/public/application.tsx

   Lines 81 to 88 in bd7d707

   	function DevToolsWrapper({
   	devTools,
   	activeDevTool,
   	updateRoute,
   	savedObjects,
   	notifications: { toasts },
   	dataSourceEnabled,
   	}: DevToolsWrapperProps) {

• devApp__container, devApp classes are identical:
  

  OpenSearch-Dashboards/src/plugins/dev_tools/public/index.scss

  Lines 8 to 17 in bd7d707

  	.devApp__container,
  	.devApp {
  	flex: 1 1 auto;
  	display: flex;
  	flex-direction: column;
  	> * {
  	flex-shrink: 0;
  	}
  	}

  
  These could be replaced by adding some properties to EuiFlexGroup and EuiFlexItem, however, OUI doesn't allow editing flex-shrink or flex-basis on EuiFlexItem.
• devAppWrapper can be easily replaced by EuiFlexGroup and EuiFlexItems using their corresponding properties like display, direction, grow:
  

  OpenSearch-Dashboards/src/plugins/dev_tools/public/index.scss

  Lines 19 to 23 in bd7d707

  	.devAppWrapper {
  	display: flex;
  	flex-direction: column;
  	flex-grow: 1;
  	}

• devAppTabs can also be replaced by EuiFlexGroup and EuiFlexItem. Although EuiFlexGroup does not have flex-flow property it can be replaced by using both direction and wrap properties, since

The flex-flow property is a shorthand property for the flex-direction and the flex-wrap properties.

OpenSearch-Dashboards/src/plugins/dev_tools/public/index.scss

Lines 30 to 34 in bd7d707

	.devAppTabs {
	display: flex;
	flex-flow: row wrap;
	justify-content: space-between;
	}

• devAppDataSourcePicker is used to add margin and min-width:
  

  OpenSearch-Dashboards/src/plugins/dev_tools/public/index.scss

  Lines 25 to 28 in bd7d707

  	.devAppDataSourcePicker {
  	margin: 7px 8px 0 0;
  	min-width: 400px;
  	}

  
  I found EuiPanel with padding-related property, but could not find margin-related property in components. We could use logicalShorthandCSS, but suppose it might still require HTML tag: https://elastic.github.io/eui/#/theming/sizing/utilities#logical

Conclusion

1. Allow EuiFlexItem to modify flex-shrink and flex-basis;
2. Replace devAppWrapper, devAppTabs with OUI library items;

[joshuarrrr]

Thanks @sayuree, this is a helpful start. I think the primary issue to address here is the fact that this plugin doesn't use https://oui.opensearch.org/1.0/#/layout/page for handling the page layout (the use of <main> is a really important hint/finding here). Note that, if we were using OuiPage* components for layout, the tab structure could easily be handled by https://oui.opensearch.org/1.0/#/layout/page-header#tabs-in-the-page-header (rather than the more generic EuiTabs), which would also mean there's no need for the spacing hacks of devAppDataSourcePicker.