Validate OUI theme compliance - phase 1

[joshuarrrr]

This is a tracking issue for phase 1 of OUI theme plugin compliance, which will involve changes to OUI typography and color SASS variables (for both dark and light mode). For full details, including dates and tooling, see opensearch-project/OpenSearch-Dashboards#4291

If your plugin has no UI views or components, please add a comment to that effect, and we'll remove the repo from future look and feel update campaigns.

Actions required

• Acknowledge the issue by adding an assignee who will serve as a point-of-contact
• Provide screenshots and navigation instructions for important views and screens in your plugin or application. This will assist with both manual and automatic validation of compliance
• Identify all typographic properties and styles
• Create issues to mitigate typographic styles not inherited from OUI components or SASS variables
• Identify all color values, including visualization colors and palettes
• Create issues to mitigate typographic styles not inherited from OUI components or SASS variables
• Resolve all mitigation issues
• Validate and test plugin with updated OUI theme

Questions?

Add a comment on opensearch-project/OpenSearch-Dashboards#4291

[wanglam]

Hi @joshuarrrr there are mainly two kind of screens in the admin UI now. Here are the screenshots:

• Deployed table list
  
  

• ML node status panel
  

We will try to find out all the typographic and styles that not inherited from OUI components, and then try to mitigate them.

[joshuarrrr]

@wanglam To answer your offline question, for:

ml-commons-dashboards/public/components/preview_panel/index.tsx

Line 100 in e2825c4

<EuiDescriptionListTitle style={{ fontSize: '14px' }}>Model ID</EuiDescriptionListTitle>

and

ml-commons-dashboards/public/components/preview_panel/index.tsx

Line 104 in e2825c4

<EuiDescriptionListTitle style={{ fontSize: '14px' }}>

The inline-styling of the font size should just be removed, and use the default sizing from EuiDescriptionListTitle: https://oui.opensearch.org/1.0/#/display/description-list

cc @KrooshalUX for visibility of guidance.

[joshuarrrr]

@wanglam Can you provide some instructions or pointers to sample data and models? We'd like to be able to load that into our testing endpoint and reproduce the views you've included here.

[wanglam]

Hi @joshuarrrr , Thank you for answering my questions. There is a model upload process in functional test. We can upload all the pre-trained models by this flow.

Here is a pure API flow to deploy a model with opensearch ml-commons 2.8 installed:

• Enable ml-commons setting

PUT /_cluster/settings
{
  "transient": {
    "plugins.ml_commons.only_run_on_ml_node": false,
    "plugins.ml_commons.native_memory_threshold": 100
  }
}

• Register model group

POST /_plugins/_ml/model_groups/_register
{
  "name":"test-model-group",
  "model_access_mode":"public"
}

Response example

{
  "model_group_id": "jL7Z24gBBvRGm7DQ4Ip1",
  "status": "CREATED"
}

• Upload pre-trained model with model group id

POST /_plugins/_ml/models/_upload
{
  "name": "huggingface/sentence-transformers/all-MiniLM-L12-v2",
  "version": "1.0.1",
  "model_group_id": "jL7Z24gBBvRGm7DQ4Ip1",
  "model_format": "TORCH_SCRIPT"
}

Response example

{
  "task_id": "n77b24gBBvRGm7DQMIrf",
  "status": "CREATED"
}

• Get model id by task id

GET /_plugins/_ml/tasks/n77b24gBBvRGm7DQMIrf

Response example

{
  "model_id": "l_Hb24gBXxK7xf32MUso",
  "task_type": "DEPLOY_MODEL",
  "function_name": "TEXT_EMBEDDING",
  "state": "COMPLETED",
  "worker_node": [
    "znWMK6yUT7mlFUTVP5SOLQ"
  ],
  "create_time": 1687315755231,
  "last_update_time": 1687315763626,
  "is_async": true
}

The model id won't be exists if task state not be COMPLETED, maybe we need to call get task API for multi times.

• Deploy model with model id

POST /_plugins/_ml/models/l_Hb24gBXxK7xf32MUso/_deploy

Response example

{
  "task_id": "w77d24gBBvRGm7DQ94pR",
  "status": "CREATED"
}

We can use the get task API to track the deploy process, the model will be list in admin UI once deploy success.

[wanglam]

@joshuarrrr There are another customize styles in the repo. Here are the codes:

• customize fontWeight

<span style={{ fontWeight: 600 }}>Not responding</span> on {planningNodesCount} of{' '}

• customized backgroundColor

<div style={{ backgroundColor: darkMode ? undefined : '#fff' }}>

There are some style properties includes padding, width and maxWidth. Do we need to mitigate these?
There are some style value in the scss file, is that means we need to change to oui variables?

[joshuarrrr]

@AMoo-Miki When you get a chance, can you confirm the pre-trained model instructions in #217 (comment) are sufficient

[joshuarrrr]

customize fontWeight

<span style={{ fontWeight: 600 }}>Not responding on {planningNodesCount} of{' '}

In general, we shouldn't be manually specifying font-weight. 600 corresponds to the $ouiFontWeightSemiBold SASS variable: https://github.com/opensearch-project/oui/blob/a0e0bb5dc3f6787d4f2872da72c57a80a7cafd6c/src/global_styling/variables/_typography.scss#L67

In this case, I'd recommend replacing the <span> tag with a vanilla <strong> tag.

customized backgroundColor

This should be replaced with an OUI component that sets the correct background automatically. I can review with @KrooshalUX and provide specific guidance.

[KrooshalUX]

It would be helpful to understand why the customization have been made - I suspect that in some cases it may be because the component was detached in Figma therefore exposing exact values instead of the OUI Sass values. In some cases it may be because the designs that were handed off had minor deviations from standards, but even in this case, the standards should always be used from OUI. I believe that @lauralexis had delivered some of these mockups prior to our internal Figma library launch, which may have lead engineering to believe optimizations should have been made.

@wanglam if you can help outline that, @lauralexis and I can provide further guidance.

[wanglam]

@KrooshalUX @lauralexis The duration picker isn't an OUI component. The background of the duration picker is white just like below image.

Since the input is read-only, the default background of FieldText is rgba(211, 218, 230, 0.05), the UI will like below image.

Could we just remove the white background and use the default FieldText readonly background?

[KrooshalUX]

@wanglam This is in regards to the auto refresh feature correct? I think I recall trying to work through this with @lauralexis during the design phase.

I am aware that the current ReadOnly state for our form controls is lacking in contrast between the active state of the form control, and perhaps that is why this customization was made to SuperDatePicker. I am wondering if instead we need to have the SuperDatePicker functionality next to an on/off switch instead? Otherwise, we should roll with the defaults for now - we have an issue open and I am working on improving the visual differentiation between the active and read only state of all form controls. If we have a customization in there, you wont get this benefit once its released.

[wanglam]

@KrooshalUX Thank you for suggesting using SuperDatePicker component. The UI of SuperDatePicker isn't 100% the same as our component. There is a min refresh rate (10 seconds) in our plugin. We will show a error tip if user input a value less than 10 seconds.

The start button will be disabled and the tip will turn red after less than 10 seconds. Seems we can't do these by using SuperDatePicker. Could you give me more suggestions about that?

[wanglam]

@joshuarrrr We have some customized color in the scss files. Is that means we need to replace it with oui variables?
Here are the codes:

• https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/monitoring/index.scss#L18
• https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/preview_panel/nodes_table.scss#L4

[joshuarrrr]

@joshuarrrr We have some customized color in the scss files. Is that means we need to replace it with oui variables? Here are the codes:

* https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/monitoring/index.scss#L18

* https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/preview_panel/nodes_table.scss#L4

Yeah, you'll definitely want to update both of those to use the OUI SASS variables. @lauralexis can you provide the corret variable?

[lauralexis]

@joshuarrrr @wanglam

The following two should be: $ouiLinkColor

• https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/monitoring/index.scss#L18

• https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/preview_panel/nodes_table.scss#L4

[KrooshalUX]

@wanglam Got it, thanks for the context refresher - its been a while since I have looked at the UI. All good to keep the higher order component you have built, please ignore my question about switching to SuperDatePicker.

[wanglam]

@joshuarrrr I've created this PR to mitigate all the typographic and color styles. Would you mind to help me review it?