Ceph » mgr » Dashboard

Following up on our discussion on the stand up, I've read about aria-labels and data-attributes.

ARIA

I found out that aria-labels must not be translated (according to its specification). It's the responsibility of the tool that reads those labels to interpret them and translate them if necessary. So that's not standing in our way to use them as selectors. But the example given in this issue might not be the best to be used as selector. It seems that aria attributes cannot completely replace the use of CSS, as, for instance, there's no ID attribute. The set of attributes and their values is limited, and I wouldn't want to use `aria-label` everywhere (as one of the attributes where any value can be assigned). The `role` attribute is limited to certain values. And overall it seems aria is designed to describe the structure of a document where the HTML tags lack the necessary description to identify their usage.

That said, I think aria-labels can be used to extend our capabilities to select elements in end to end and unit tests. It might haven't dug deep enough to find a proper application of aria attributes to completely replace CSS selectors, though, so that's something I cannot completely exclude, but it seems to be unlikely.

Data attributes

Data attributes could basically be used everywhere in our application and replace CSS classes and IDs. What I don't like about the approach is, that they've been invented so serve a different purpose. Data is supposed to be added to HTML tags for JavaScript application to retrieve. JavaScript applications can store data there, too, to be retrieved by sub components (although I don't think that's clean). Using this technique the backend could provide data to the frontend using HTML, provided that the backend is not an API that just returns data but also returns templates.

Though, using data attributes as selectors would likely not interfere with anything we currently do and provide a clear distinction between CSS classes and IDs for styles and selectors purely used for testing purposes.

Summary

My personal impression is that data-attributes, although the could be used to solve this problem, also have the potential to complicate things. When refactoring code (especially templates), I already have to take care of classes, so that styles won't change. With the addition of data attributes, I'd have to take care of both, classes and data attributes used as identifiers for tests. Classes are not so likely to be changed IMO, especially when've been chosen good.

Some of the arguments in the description of this issue are examples for bad practices:

CSS classes might change with framework updates (e.g.: the recent Bootstrap 3 to 4 refresh) or simple cleanups.

We should never rely on framework classes to select elements, neither in tests nor in CSS. Instead, we should add own descriptive CSS classes to HTML elements.

Adding new elements with the same attributes would modify/break the query results.

That is expected. If the tests expect two elements of a certain kind and it finds three, it's supposed to fail and be adapted not to fail. If the test fails due to another problem, it's likely that the selectors used in those tests have poorly been chosen. The same can happen to data attributes which are used in tests and added to the template at a later point in time.

Angular component attributes include literal strings that might change ("Cluster Status" -> "Cluster Health").

I honestly think we should never select elements based on their content.

ARIA attributes seem to provide the ability to reduce the necessity of classes, though, but not replace CSS classes.

Thanks for your thorough feedback, Patrick!

As discussed by IRC, this is not meant in any way to replace CSS classes or IDs. This is strictly focused on making front-end testing easier and more maintainable.

Based on the HTML5 standard, data attributes are purposed for:

Custom data attributes are intended to store custom data, state, annotations, and similar, private to the page or application, for which there are no more appropriate attributes or elements.

They're purely private/custom use.

I'm not an expert on front-end testing, but every time I've had to tackle that I found myself doing kind of the following:

# Python equivalent of a front-end unit test

variable_to_test = Fixture.give_me_the_variable_that_is_below_another_variable_named("weird_variable_name").count(3).inside_if_block(True).count(2)

By introducing data attributes with unique IDs, we allow a developer to quickly fetch and test a specific sub element of a component without needing to navigate through the HTML DOM, or any other structural/presentation concern.

tldr;

I do not insist on having to use data-attributes solely for testing and not using classes or tags for selecting elements, but that's what the author of the article states, which you've added, is the benefit of this approach. And I don't see another one. IDs and CSS classes are as good as data-attributes to solve the problem, but the former do not add more complexity for a probably small benefit.

end of tldr;

By introducing data attributes with unique IDs, we allow a developer to quickly fetch and test a specific sub element of a component without needing to navigate through the HTML DOM, or any other structural/presentation concern.

This is basically replacing the HTML attribute `id` with data attributes. `id` can be very useful but should also be used sparely, as it is only valid to have a single unique id on a single page. By using classes, this problem can be resolved.

Let's just use CSS selectors when talking about selection issues, as they are very popular, not only in CSS. There's sizzle, the JS selector engine (used in jQuery) and todays browsers support the same funtionality using `document.querySelector` respectively `document.querySelectorAll`. So everything speaks the same language here.

By adding data attributes or by using ARIA attributes, we extend the capabilities to select elements here, possibly reducing the complexity of the CSS selector. The same can be done by adding classes. So the whole benefit I see here for data-attributes, is that we might have a clear distinction between identifiers and their purposes. The common classes and IDs are for CSS purposes only, data attributes would be used solely for testing purposes.

Now, let's talk about the selection issues.

give_me_the_variable_that_is_below_another_variable_named

<div class="box box-purpose">
  <ul>
    <li>one</li>
    <li>
      <ul>
        <li>two</li>
        <li>three</li>
      </ul>
    </li>
  </ul>
</div>

Let's assume I want to select the second list, but I dont' want to use any n'th element style selectors (which I recommend to almost always avoid).

.box ul ul {}

returns the second list. But what if I wanted to select the first list only?

.box ul

doesn't solve that problem, as it selects both lists.

.box > ul

does select the list thats immediately below the selected box, so the second list is excluded.

Without having do add any more classes than the first one, I was already able to select the list I wanted. Surely, I could add classes or data-attributes to it. Let's do that and see how it'd look like.

<div class="box box-purpose">
  <ul class="list-purpose">
    <li>one</li>
    <li>
      <ul class="list-sub-purpose">
        <li>two</li>
        <li>three</li>
      </ul>
    </li>
  </ul>
</div>

The selection of the first and the second list has been simplified to either

.box .list-purpose {}
/* or */
.box .list-sub-purpose {}

What would that look like with data attributes?

<div class="box">
  <ul data-purpose="list-purpose">
    <li>one</li>
    <li>
      <ul data-purpose="list-sub-purpose">
        <li>two</li>
        <li>three</li>
      </ul>
    </li>
  </ul>
</div>

.box [data-purpose="list-purpose"] {}
/* or */
.box [data-purpose="list-sub-purpose"] {}

But that's mixing up CSS classes and data-attributes. If we want to have the benefit of the distinction between data attributes and CSS classes, we would need to do the following.

<div class="box" data-purpose="box">
  <ul data-purpose="list-purpose">
    <li>one</li>
    <li>
      <ul data-purpose="list-sub-purpose">
        <li>two</li>
        <li>three</li>
      </ul>
    </li>
  </ul>
</div>

[data-purpose="box"] [data-purpose="list-purpose"] {}
/* or */
[data-purpose="box"] [data-purpose="list-sub-purpose"] {}

Maybe we'll need more CSS classes for styling, too, so that we'd end up with having data-attributes and css classes on the same elements. The HTML code IMHO will eventually look less clean. The benefit we had with the distinction of the identifiers for styling and testings might increase complexity.

The selectors used in testing are different, too, which when I try to image having to use them all over the place, is not necessarily a benefit. I'd restrict myself to add data-attributes everywhere to not having to use CSS classes or tags as selectors, as that could defeat the purpose of data attributes (at least for what we'd use them), namely preventing the tests to break when the templates (CSS and tags) are refactored.

I do not insist on having to use data-attributes solely for testing and not using classes or tags for selecting elements, but that's what the author of the article states, which you've added, is the benefit of this approach. And I don't see another one. IDs and CSS classes are as good as data-attributes to solve the problem, but the former do not add more complexity for a probably small benefit.

I want to conclude with a small example of how it can be prevented to select nth-elements. The Block -> Mirroring page of the dashboard contains three tables. The test needed to check for an element (row) inside the second table and did so by selecting the second element of the result, like so:

$('cd-table')[1]

which can easily be prevented by adding classes to the tables:

$('.table-mirroing-pools')

or even by choosing a better selector:

$('table-mirroring-pools cd-table')

I cited that article just for the use of data attributes in testing, not for a full replacement of CSS. Both data attributes, CSS classes or ARIA labels can be used as selectors in CSS stylesheets (e.g.: Patterfly uses aria-invalid for styling input elements with invalid data, instead of relying on ng-invalid or any other framework specific class), but I wasn't advocating for that.

I don't have any strong preference for data attributes over ARIA labels. What I'd rather avoid is to use CSS classes, which are representational tools (they map HTML DOM to the CSS presentation layer), for testing purposes.

The use of CSS IDs was suggested by Adam & Rafael, and they were discouraged from that, because of some side effects (uniqueness, use in forms, anchors, etc).

I'll add more specific examples for my point:

• Let's say I want to relabel an input from class "form-control" to "pf-form-control". That makes a test fail. IMHO no test should fail because of a styling change.
• We avoid the need of doing: const purge = fixture.debugElement.query(By.css('.table-actions button .fa-times')), when we can simply do const purge = fixture.debugElement.query(By.css('[data-testid="purge-button"]'). Yes, we could use IDs here, but we discarded the idea of using IDs, don't we?
• Our custom CSS classes don't follow any convention: we mix crush-rule-info, which is kind of semantic, with oa-hr-small, which is purely presentation-aware.

In my opinion, by using ARIA labels/data attributes, we can label HTML/DOM elements, the same way as we name a variable we want to test, with no other side-effects/concerns (the latter is more applicable to data attributes, than to ARIA labels, which might bring side effects).

About the data attributes. I just wanted to raise my concerns and I still have them, but I also think I shouldn't try to make my point any clearer as this would inevitably result in another big reply (I tried!). It is just my gut feeling that we don't necessarily need the implementation but I also don't want to prevent it by all means. It would work, I just don't feel the need to have it implemented. And although I still have my concerns, it might be a good idea.

Using ARIA attributes is surely a good idea as it also serves its own purpose.