Skip to content
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] OUI Usage Audit Example home plugin #3945

Open
BSFishy opened this issue Apr 25, 2023 · 1 comment
Open

[Research] OUI Usage Audit Example home plugin #3945

BSFishy opened this issue Apr 25, 2023 · 1 comment
Labels
OUI compliance Issues and PRs to maximize OUI usage and remove style and component hacks OUI Issues that require migration to OUI

Comments

@BSFishy
Copy link
Contributor

BSFishy commented Apr 25, 2023

#4111 for tracking

Overview

OUI is the component library that Dashboards and all its plugins use to maintain a consistent look and feel. Dashboards and its plugins, however, do not use OUI consistently. This can lead to situations where OUI updates a component, which might end up breaking functionality in Dashboards because functionality is overridden. A situation could also arise when OUI updates a component but Dashboards doesn't receive it because it doesn't use that component at all.

This audit is meant to analyze the usage across Dashboards and attempt to remedy misuse of OUI through changes to either Dashboards or OUI. Specifically, we are looking for usage of custom CSS, where OUI either isn't used or needs to be overridden to achieve the desired functionality.

Home Plugin Audit

I performed a preliminary audit to the Home plugin to understand how the rest of this audit should go. Here are my findings:

  • Some custom classes are defined in the HTML but not in CSS. For example, a component might add homSampleDataSetCard to its list of classes, but no styles will be associated with that class.
  • A lot of the custom CSS relates to styling.
    • This is things like applying a flexbox, margin, width, height, etc.
    • It’s possible that if these stylings are deemed reusable, they could be integrated in upstream OUI
    • Wherever specific values were needed, OUI variables were used, for example $euiSize, $euiSizeXL, etc.
      • The styles will react to theming as long as the theme exposes the correct variables (which all proper themes should), because those variables are what is used to determine specific values
  • A good amount of the custom styling comes from 2 sources
    • Welcome screen
      • This is the initial screen when you start Dashboards, welcoming you and asking if you want to add data
      • Most of the styling is for layout
      • As far as I’m aware, there is no component in upstream OUI that provides this sort of layout
      • Not sure if it’s possible to achieve this sort of layout using composition
      • I don’t think it’s worth adding a component that can do this sort of layout to OUI if it’s only used in one place
    • Solutions panel
      • This is the large panel on the Home page
      • It extends OUI panel
      • There is no upstream OUI component that provides this sort of layout
      • I believe @KrooshalUX wants to either change the design to align with upstream OUI or somehow integrate the current design into OUI, however I could be wrong about that
  • Apart from the 2 sources above, most custom classes are 1-3 styles. Ideally, these could be put into upstream OUI under a new prop or even by modifying existing props
  • While no change is necessary, it would be ideal if Dashboards didn’t have any custom styles and OUI provided everything Dashboards needed through an existing component or through composition

Auditing the Rest of Dashboards

The initial audit of the Home plugin was good and has provided us with a few action items to remedy uses of custom CSS, however it only applies to the Home plugin. We need to do the same audit to the rest of Dashboards.

Here is a rough set of steps to perform an audit:

  1. Define your area of impact. This is the set of files you plan to be auditing. I recommend doing a plugin at a time.
  2. Take a breadth-first approach and identify files that will be appropriate for the audit. For example, files related to creating mocks might not be as relevant as component files.
  3. Once you've identified files to audit, begin taking a brief look at them to identify if they're still relevant. Files that don't define any custom CSS or use any custom classes aren't very relevant.
  4. Once you have a list of files that have custom CSS, look more in depth at what exactly they're trying to achieve with the custom CSS. Is it something that OUI is able to provide?
  5. Publish your findings so that we can fix the issues.

Here is each step in more depth:

1. Define your area of impact

It's important to define a scope ahead of time so you're not trying to audit the entirety of Dashboards in one go. Keep it small and simple so that you don't get burnt out doing the audit and so that we have small, actionable outcomes to the audit.

Ideally, we're auditing a plugin at a time. For example, I started with the Home plugin. Next steps may be the Dashboard plugin or Visualize plugin. For much larger or complex plugins, it may make more sense to keep the scope smaller. Take it case-by-case and use your best judgement.

Side note: Make sure no other audit has been done on what you're trying to audit. For example, the home plugin has already been audited. There is no point in auditing it again. A way to check if a plugin has been audited is by simply searching the issues to see if an issue has been created to audit the plugin your looking at.

2. Identify files that will be appropriate for the audit

Since we're taking a breadth-first approach, it's important at this point to make a list of files that may be useful to the audit. We don't need to audit server-side code or code related to testing, or anything like that. We're looking specifically at front-end code. In most cases, this means we're looking specifically at the public/ folder in the plugin. However, it's important to deep dive into each plugin! Take it on a case-by-case basis.

When I was auditing the Home plugin, I identified that I didn't need to look at anything in the server/ or common/ folders. Beyond that, I identified that public/assets/, public/mocks/, and public/services/ were all irrelevant too. By ruling out irrelevant files, I was able to make sure I wasn't wasting any time looking at files that wouldn't contain custom CSS anyway.

3. Take a brief look at files to identify if they're still relevant

This is the point where we start actually looking at the code. Again, we're taking a breadth-first approach, so it isn't imperative to understand the code inside and out. We're just looking for custom styles in Javascript files, and the existence of Sass files.

In Javascript files, we're specifically looking for native HTML tags and className attributes. For example, you can see in this file in the Home plugin, a custom class is used:

<EuiFlexGrid columns={3} className="homSampleDataSetCards">

Not all cases of className are valid, however. OUI provides a couple utility classes that are supported, such as in this example:

<EuiTextColor className="euiText--small" color="subdued">

Beyond that, native HTML tags are usually a good indicator that something is being done without the help of OUI. Some examples of native HTML tags could be span, img, div, etc. An example of this would be something like this from the Home plugin:

<div className="homSolutionPanel__customIcon">
<img
className="homSolutionPanel__customIconContainer"
data-test-subj="dashboardCustomLogo"
data-test-image-url={customLogo}
alt={branding.applicationTitle + ' logo'}
src={customLogo}
/>
</div>

However, again, not all cases of native HTML elements are valid for this audit. Some OUI components have the user provide a native HTML tag for accessibility reasons or otherwise. For example, OuiTitle expects its child to be another component to update for accessibility reasons. This example is perfectly valid:

<EuiTitle
className="homSolutionPanel__title eui-textInheritColor"
size="s"
data-test-subj="dashboardCustomTitle"
data-test-title={branding.applicationTitle}
>
<h3>{branding.applicationTitle}</h3>
</EuiTitle>

In most cases, you should be able to just skim through the file and look for any native HTML tags or className attributes.

All in all, take it in a case-by-case basis. It's better to cast your net wide and remove things later on than to miss out on something. Err on the side of caution, we're trying to get as much info from these audits as possible.

4. Look at what the custom styles are trying to achieve

The final step is understanding why custom styling is being used. This is the point where you should start putting together some sort of analysis, like I did in the "Home Plugin Audit" section above. We really want to know why custom styles are being used and where there are gaps in OUI.

A list of action items should be compiled. Whether it's making changes to Dashboards to support OUI components, updating OUI to support different features, or whatever. There should be some sort of outcome to each audit unless no custom styling is used whatsoever.

5. Publish your findings

The outcome of an audit is a set of action items. For this one, we're trying to remove custom styling from Dashboards. Action items should be in 2 categories: removing custom styles in Dashboards and remediating gaps in OUI.

Removing custom styles in Dashboards is quite straight forward. Simply create an issue where the custom styles are and potentially add some recommendations on how to OUI-ify it. If you have no idea, feel free to create an issue in OUI for guidance or support.

Identifying and remediating gaps in OUI is a little bit more involved. This is the case if you need something in Dashboards that OUI doesn't provide. In this case, we'll need to modify OUI to support this new functionality. Create an issue in OUI about the specific situation, and we'll work to provide you with the best solution we can. As a little bit of insight, OUI is meant to be a generic component system, not just for Dashboards, so we need to make sure the components are generic enough and that the UX is good enough to hit that target. That's why making changes in OUI is a little more involved than in Dashboards.

@BSFishy BSFishy changed the title OUI Usage Audit [Research] OUI Usage Audit Apr 26, 2023
@seanneumann seanneumann added the OUI Issues that require migration to OUI label May 18, 2023
This was referenced May 22, 2023
@joshuarrrr
Copy link
Member

@BSFishy Do we still need to generate actual recommendations for the home plugin? I don't see any linked mitigation issues yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OUI compliance Issues and PRs to maximize OUI usage and remove style and component hacks OUI Issues that require migration to OUI
Projects
Status: In Progress
Development

No branches or pull requests

3 participants