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

Composition of Interactor filters and locators #773

Closed
cowboyd opened this issue Jan 15, 2021 · 1 comment
Closed

Composition of Interactor filters and locators #773

cowboyd opened this issue Jan 15, 2021 · 1 comment
Labels
@bigtest/interactor higher order interface manipulation

Comments

@cowboyd
Copy link
Member

cowboyd commented Jan 15, 2021

One of the critical features of interaction actions is the ability to compose them. We can see this in action for the FOLIO Select interactor which operates on a corresponding Select component that has roughly the following HTML structure:

<div>
  <label></label>
  <select></select>
  <div class="error"></div>
  <div class="warning></div>
  <span class="labelText"></span>
</div>

As we can see, this component is just a value add on the native HTML <select> tag in order to make it aware of form level concerns like errors, warnings, and to make sure that it has a label so that the user can identify it.

The definition of the select interactor is as follows:

import { createInteractor, Select } from '@bigtest/interactor';

export default createInteractor('select')({
  selector: '[class^=select-]',
  locator:  el => el.querySelector('[class^=labelText]').innerText,
  filters: {
    id: el => el.querySelector('select').id,
    label: el => el.querySelector('[class^=labelText]').innerText,
    ariaLabelledBy: el => el.querySelector('select').getAttribute('aria-labelledby'),
    placeholder: el => el.querySelector('select').getAttribute('placeholder'),
    value: el => el.querySelector('select').value,
    error: el => {
      const feedbackError = el.querySelector('[class^=feedbackError]');
      return feedbackError ? feedbackError.innerText : undefined;
    },
    warning: el => {
      const feedbackWarning = el.querySelector('[class^=feedbackWarning]');
      return feedbackWarning ? feedbackWarning.innerText : undefined;
    },
    valid: el => el.querySelector('select').getAttribute('aria-invalid') !== 'true'
  },
  actions: {
    choose: (interactor, value) => interactor.find(Select()).choose(value)
  }
});

Contrast the elegance of the choose() action vs the clunkiness of the various filters. There are a couple of things to note. Most of the filters are just delegating to what would be the filters of the contained element. For example, the locator of the select is really just the locator of the label element, and placeholder filter is just the placeholder filter of the contained selected element. Not only to we have to duplicate the logic, but we also have to do it in an unsafe synchronous context which results in a bunch of inscrutable no textContent on 'null' errors if they aren't present.

It would be nice if there were some way to compose filters in the same way that we do interactors:

filters: {
  error: interactor => interactor.find(Select()).filters.text()
}

Where text() would be an async function that resolves to the value of the text fillter. The biggest advantage is that all of the operations would be protected, no matter how far you went down so that if the element wasn't found, you'd know exactly why rather than getting what amounts to a null pointer exception.

The main question would be how to reconcile this with the synchronous syntax which is very convenient.

Potential API

One way would be to follow the lead of the actions api where a filter would become an async function that resolved to a value instead. Corresponding to the perform() method of actions, which changes state, there would be a get() that resolves a value from an Element only once that element matched all of the lookup filters

// normal 
(interactor) => interactor.get(el => el.getAttribute('placeholder'))

This could then be used to delegate the above placeholder filter so that this

el => el.querySelector('select').getAttribute('placeholder')

Can be written as:

(interactor) => interactor.find(Select()).get(el => el.getAttribute('placeholder');

Another question: what happens if we want to use the actual filter of the contained select in the same way that we can use the actual action of a contained interactor? In other words, the Select interactor has its own placeholder filter. Rather than implementing the call to getAttribute() in both parent and child, it would be nice if we could just delegate straight to the child. However, to do so, we would need to make all of the filters available on a .filters property:

(interactor) => interactor.find(Select()).filters().placholder()

since filters never take arguments, we could potentially drop the method syntax:

(interactor) => interactor.find(Select()).filters.placeholder

This also has an interesting application in actions in that we can now use filters inside them for branching logic. Take the case in the FOLIO dropdown interactor where the actual floating menu can live either inside the main dropdown element or in a completely separate layer. Writing the select action was difficult because it was difficult to tell where the subelement would be. If we could use a filter inside of an action, then this would be really easy and straightforward:

async function choose(interactor, value) {
  if (await interactor.filters().floating()) {
    await Layer(0).find(MenuItem(value)).click();
  } else {
    await interactor.find(MenuItem(value)).click();
  }
}

Implementation Issues

  • There is the risk of "circular" filters which reference each other in their own definitions causing infinite loops.

Naming

Part of me wonders if this api would be accurately called "filters" since a filter is more of a value that you provide to select (or reject) against as set of attributes. Maybe what we really want is an attributes api, and then filters which match on attributes:

.attr('visible', ({ get }) => get(isVisible))
.attr('placeholder', ({ get }) => get(el => el.getAttribute('placeholder'))

and filters matching atributes:

let filters = { visible: true, placeholder: 'Save' };
Button(filters).click();
@cowboyd cowboyd added the @bigtest/interactor higher order interface manipulation label Jan 15, 2021
@cowboyd
Copy link
Member Author

cowboyd commented Jan 15, 2021

After discussion. It seems like this path is fraught with danger. The biggest issue is that in order to maintain the promise of interactors, filters must be synchronous. We have to be able to evaluate each and every filter in a single tick of the event loop while we know that the element is present, and we can also ascertain the values of every other filter at the same time.

While the current API can be awkward at times, we concluded that in order to address it, it would require us to construct a parallel function composition API similar to the Promise api and async function api that worked synchronously instead of asynchronously. Rather than wade into those eel-infested waters, we decided to accept, for the nonce, the current idiosyncrasies of the filter declaration API as being known limitations rather than embark on a new research project with an uncertain outcome.

Instead, we're going to focus on making filters easier to use rather than making them easier to define with the idea being that they will be written once, but be used to lookup interactors and make assertions many, many more times. If we can make that experience really great, then it will be a better amplification of our efforts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@bigtest/interactor higher order interface manipulation
Projects
None yet
Development

No branches or pull requests

1 participant