-
Notifications
You must be signed in to change notification settings - Fork 893
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
Labels
OUI compliance
Issues and PRs to maximize OUI usage and remove style and component hacks
OUI
Issues that require migration to OUI
Comments
This was referenced May 22, 2023
This was referenced May 22, 2023
joshuarrrr
changed the title
[Research] OUI Usage Audit
[Research] OUI Usage Audit Example (May 24, 2023
home
plugin)
joshuarrrr
changed the title
[Research] OUI Usage Audit Example (
[Research] OUI Usage Audit Example May 24, 2023
home
plugin)home
plugin
joshuarrrr
added
the
OUI compliance
Issues and PRs to maximize OUI usage and remove style and component hacks
label
Jun 1, 2023
@BSFishy Do we still need to generate actual recommendations for the |
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
#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:
homSampleDataSetCard
to its list of classes, but no styles will be associated with that class.$euiSize
,$euiSizeXL
, etc.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:
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.
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/
orcommon/
folders. Beyond that, I identified thatpublic/assets/
,public/mocks/
, andpublic/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:OpenSearch-Dashboards/src/plugins/home/public/application/components/sample_data_set_cards.js
Line 200 in 66aa122
Not all cases of
className
are valid, however. OUI provides a couple utility classes that are supported, such as in this example:OpenSearch-Dashboards/src/plugins/home/public/application/components/welcome.tsx
Line 255 in 66aa122
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:OpenSearch-Dashboards/src/plugins/home/public/application/components/solutions_section/solution_title.tsx
Lines 107 to 115 in 66aa122
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:OpenSearch-Dashboards/src/plugins/home/public/application/components/solutions_section/solution_title.tsx
Lines 148 to 155 in 66aa122
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.
The text was updated successfully, but these errors were encountered: