Components: CDK - HarnessPredicate: override or disable base options

[kreuzerk]

Feature Description

Option to disable or override the base options of a HarnessPredicate to keep our Harness implementation consistent.

Use Case

We have a component library and in some cases the FormControl is wrapped inside a custom component. For accessibility reasons we still want to link the label with the underlying FormControl. To do so, we need to remove the attribute reflection on the Host element to prevent having multiple Ids on the page.

// This code prevents the attribute reflection of the Id attribute into the DOM
@HostBinding('attr.id') null;

The problem is, that this breaks the lookup via selector on the base options. We would need to change the lookup to check the FormControl instead of the Host.

Of course, we could provide custom HarnessFilters. But then we would need to rename the options. It would be nice if we could override or disable the base options. Disabling or overriding the base options would allow us to keep a consistent interface throughout all components in our UI framework. Basically every Harness object would always provide the selector plus maybe something additional.

Possible solutions / approaches

Pass in custom match function

The HarnessPredicate could accept a custom match function

  constructor(public harnessType: ComponentHarnessConstructor<T>, 
             options: BaseHarnessFilters,
                     matchFunction?: ((option: BaseHarnessFilters) => Promise<boolean>) | null) 
{  }

Option to disable the base functions and then use the addOption to provide them yourself

  constructor(public harnessType: ComponentHarnessConstructor<T>, 
              options: BaseHarnessFilters,
                      disableBaseFunctions = false)
  {}

and then inside our Harness implementation:

 static with(options: BaseHarnessFilters): HarnessPredicate<CustomHarness> {
    return new HarnessPredicate(CustomHarness, options).addOption(
      'custom selector option',
      options.selector,
      async (harness: CustomHarness, selector: string) => {
        if (selector.startsWith('#')) {
          const inputField = await harness.getInputFieldElement();
          console.log(`#${inputField.getAttribute('id')}` === selector);
          return HarnessPredicate.stringMatches(
            `#${inputField.getAttribute('id')}`,
            selector
          );
        }
        return (await harness.host()).matchesSelector(selector);
      }
    );
  }

Let me know what you think of those proposed solutions. Is this something that you consider? If yes, I would be happy to open a PR.

[devversion]

It sounds reasonable being able to override the base options, though I'm unsure what's the best approach here.

The base harness filters also typed to support e.g. selector, so providing an option to disable them (as per your snippets) could cause ambiguity with that options interface. At first glance, it seems like the best solution would be to just make the options parameter optional, or null-able. Based on that we could then detect whether we should provide the base options or not.

cc. @mmalerba

[mmalerba]

Do we actually need to change anything to support this? Why don't they just not pass the options through in their with function, like this:

static with(options: BaseHarnessFilters): HarnessPredicate<CustomHarness> {
    return new HarnessPredicate(CustomHarness, {})
        .addOption('custom selector option', options.selector, (...) => {...})
}

Or is there something I'm missing?

[kreuzerk]

@mmalerba you are right, this solves the issue. Thanks a lot for this solution!

Even though, I think that having optional options is would be a more expressive API. In my opinion, it would be easier to understand that the options can be optional. Currently, you can only get to this solution by reverse-engineering the code.

Still, I think my use case is an edge case and your proposed solution works for me. Therefore I am going to close this issue. @mmalerba @devversion Thanks for the help 👍

[devversion]

Do we actually need to change anything to support this? Why don't they just not pass the options through in their with function, like this:

Yeah, sounds like you're right. I was thinking of supporting an explicit null, but didn't realize that one could just pass in
an empty object literal. That seems perfect as it allows developers to override specific options (and not all base options; considering there would be more obviously)

Even though, I think that having optional options is would be a more expressive API

Yeah, I'd agree with that. Though if we make it optional it might cause people to forget about the base options that could be supported by the harness predicate.

[mmalerba]

I would prefer to keep it non-optional so that people don't forget to pass it along as @devversion mentioned

[kreuzerk]

Agree. My case is anyway an edge case and passing an empty object works fine 👍

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{_This action has been performed automatically by a bot._}