Visibility-Checks on Elements with display:contents

[mirobo]

Before I create an issue I wanted to check what Cypress' position is on the following topic.

In Angular some components often have host style with "display:contents" so that the layout is not changed by the element of the component (I'm not aware of other reasons other than lazyness when layout issues occur?)

Now in the tests there may or may not exist selectors which point to such an element which has set "display:contents". This selector may then be used in an assertion like cy.get('myElementWithDisplayContents').should('not.be.visible') and a counter part that checks for visibility may not exist. We cannot easily know whether that other assertion exists.

What we're left with is an assertion that ALWAYS passes because an element with "display:contents" is always invisible to Cypress (and also the inspector in the browser). There may be really strange edge cases where some JS dynamically changes "display:contents" on and off. But I think it would be really advantageous for the majority of Cypress users to let the test immediately fail whenever a selector points to an element which has "display:contents" set.

I'm aware that this means that more checks need to be done when running assertions like "not.visible" / "not.be.visible" (that's what I did in a class which wraps cy.get().

What is Cypress' position on this? Would it be possible/feasable/smart to add this check into Cypress and let the user deactivate it for edge cases?

Personally I think I could live with my workaround even though it's not really pretty and efficient.

class WrapperExample {

  rootElement: () => Cypress.Chainable;

  constructor(rootSelector: string) {
    this.rootElement = () => cy.get(rootSelector);
  }

.. other functions omitted

  // ensure that a selector is not set on an element with display contents when not.visible is assert
  // because it is always hidden
  ensureElementHasNoStyleDisplayContents() {
    cy.log('NoDisplayContents');
    // check that the element does not contains "contents"
    this.rootElement().should('satisfy', ($el: JQuery<HTMLElement>) => {
      const cssDisplayValue = $el.css('display') ?? '';
      const classList = Array.from($el[0].classList);
      console.log(`cssDisplayValue: ${cssDisplayValue}`);
      console.log(`classList: ${classList}`);
      const hasStyleDisplayContents =
        classList.includes('contents') || cssDisplayValue.toLowerCase().includes('contents');
      return !hasStyleDisplayContents;
    });
  }

  notVisible(): this {
    this.ensureElementHasNoStyleDisplayContents();
    this.rootElement().should('not.be.visible');
    return this;
  }

  should(chainers: string, value?: any): this {
    if (chainers == 'not.be.visible' || chainers == 'not.visible') {
      this.ensureElementHasNoStyleDisplayContents();
    }
    this.rootElement().should(chainers, value);
    return this;
  }
}

In parallel I'm working on more strict typing for selectors so that the above can be avoided during compile time:

"my-autocomplete-component" could be component which has "display:contents" and contains an input element which actually can be dynamically shown/hidden:

class AutocompleteInputWrapper {

  constructor(rootSelector: `my-autocomplete-component[data-cy='${string}'] input`) {
  ...
  }
}

This way I know in my wrapper class what kind of component was selected and where the selector is pointing at..

Otherwise it's just a game of guessing and requires other code such as:

export class ButtonWrapper {

  buttonElement: () => Cypress.Chainable;
  rootElement: () => Cypress.Chainable;

  constructor(rootSelector: string) {
    this.rootElement = () => cy.get(rootSelector);

    buttonElement = this.rootElement().then((el) => {
        // we don't know where rootSelector was point at.. was it an element/wrapper element which contains a button?
       // or was it the button itself? we cannot tell, so we search for it
        if (el.prop('tagName') === 'BUTTON') {
          return this.rootElement() as Cypress.Chainable<JQuery<HTMLButtonElement>>;
        } else {
          return cy.get('button:first', { withinSubject: el }) as Cypress.Chainable<JQuery<HTMLButtonElement>>;
        }
     });
  }


  ...
}

Any ideas :D

[mirobo]

@lmiller1990 Would it be possible to have Cypress' position on this one?

I can also explain why in our (real world) app we still need to have wrapper classes for several UI components and also page objects:

• We need to respect the DRY principle (even though Cypress suggests not to care that). I write mostly wrapper classes so developers can focus on the test of a specific use case and they don't need to know how to operate an autocomplete component or a custom select component. Not respecting the DRY principle would make our testcode completely unmaintainable to say the least.

• We don't always know if the usage of a component in a use case is correct. Strict template checking and eslint simply won't make up for man-made errors. I encourage developers to refrain from re-testing "low level" UI component functionality. But in some cases it is still necessary because the UI components are quite flexible and are NOT full of logic / magic.

• We test the app with mocked services. But in a later phase we need to test it all together with the actual services from other applications and for this case we re-use the page objects and wrapper classes for UI components. We know that we could re-use complete tests, but the stuff that we check is very different in that later phase. So we re-use just the UI elements and page objects that we created.

• Not all tests are "complete". An element may be checked for invisibility but not for visibility. It's tricky to figure out if the counter-part was tested (code coverage reports will help with that a bit)

I probably could go on... ;-)

[lmiller1990]

Hi @mirobo, terribly sorry for the slow response on this one - I was out of office for all of October, but I'm back now.

Regarding visibility in Cypress, not sure if you've seen it, but the algorithm is described here. I see the problem you are describing - display: contents removes the element from the DOM tree as noted here.

It looks like this behavior is actually a browser bug, according to MDN. display: contents should be visible. From an a11y point of view, the element is NOT visible, which means the current behavior is (somewhat) desirable, but I understand your side of the argument, too.

Would a cy.existsInDomTree-like selector actually be more appropriate to express what you are trying to in your tests?

As for actually changing the visibility algorithm in Cypress, I think it's hard to make a case for this, mainly due to the a11y angle - our selectors should respect both regular users and those using things like screen readers.

Happy to keep discussing this though and get more input. Did anything I say resonate with you?

[mirobo]

Sorry for the (very late) reply. I'm not quite sure how cy.existsInDomTree() would solve the issue because it's something I'd like to tackle in TypeScript. It wasn't "tackable" in Java with Selenium/LeanFT at all, but in TypeScript I see some options at least :D

Of course I can throw errors at runtime if the selector does not point to the desired element but at that point it's already too late. I probably need narrow types to make sure that a proper selector is set for a specific wrapper class.

Going with Cypress.Commands.addQuery would probably lead to cleaner solution than the one I already have.

So I could have the custom query cy.getButton('some-selector') which yields always a button element. If the selector does not point to a button element, I'll grab the first button inside the actual element.

Just to be clear: In an ideal world I could extend the Cypress interface with custom commands in such a way that I don't need my wrapper classes anymore. One very important aspect of those wrapper classes is excellent IntelliSense "support" by using method chaining in the wrapper classes. Probably I just need to experiment with a different approach...

Generally I think it would be nice for you to see how the majority of users write test code with Cypress. I wish I could go without any custom wrappers.. but reality says otherwise :D

[lmiller1990]

how the majority of users write test code with Cypress

I've used Cypress at many other places - my experience it is really different, depending on the team, test authors, etc. We should support all use cases, and be as flexible as possible.

I just re-read this thread - is there anything actionable here? I understand why you want display:contents to fail automatically. I think this change would be considered breaking, though, which means it would require quite a lot of discussion and consideration, along with a migration strategy, risk mitigation, etc. It's difficult to justify this change considering how long and widely used the current visibility algorithm is.

I also don't think a configuration option is ideal, Cypress is already far too complex and configurable - there are over 100 potential configuration options (way too many, imo - if anything, I wish we could simplifying and reduce the API and surface area).

If I was rewriting the product, or creating a new one, I think this change would make sense, but less so for a mature tool.

What do you think? Is there anything else actionable on our end?

[mirobo]

I see your point that it requires a lot of work to make such a drastic change and tbh it doesn't contribute much to the overall capabilities of Cypress. We all know that everyone of us never worked with ANY piece of software that didn't require special hacks/workarounds, so I guess it helped to realize that once again and live with my current solution :D

Recarding further actions, see this independent issue #25199
What could come out of this could be a flag on how to treat "display: contents".. but again.. I accept my fate 😁 and nonetheless I'm very happy overall with the current feature set. Once my private PC works properly with Cypress I might dive into it again..

The TypeScript stuff I need to figure this out by myself - it seems that not many people care about compile-time support for correctly set selectors, and fair enough, it was a problem in every automation software before and it's nothing new 😅

If this discussion is "closeable" you can close it.. and thanks again for your time!

[lmiller1990]

I think a blog post detailing how you handle this and your general ideas around type safe selectors would be great. If you write one, please share it with me! We could also retweet on the Cypress blog and account.