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

Feature: before and after hooks #454

Open
artemgurzhii opened this issue Apr 30, 2019 · 10 comments
Open

Feature: before and after hooks #454

artemgurzhii opened this issue Apr 30, 2019 · 10 comments

Comments

@artemgurzhii
Copy link
Contributor

We have input fields on which validation runs after focus-out, and because of that I need to write tests like this:

module('Integration | Component | login-form', function(hooks) {
  setupRenderingTest(hooks);

  test('validate inputs', async function(assert) {
    await render(hbs`<LoginForm />`);

    await page
      .email('')
      .blurEmail()
      .password('')
      .blurPassword();

    assert.ok(page.submitDisabled, 'Submit button is diabled');

    assert.equal(
      page.emailError,
      "Email can't be blank",
      'Shows error when email is blank',
    );

    assert.equal(
      page.passwordError,
      "Password can't be blank",
      'Shows error when password is blank',
    );
  });
});

Is there any way to have after option on the fillable helper, with following API:

/**
 * @param {string} selector - CSS selector of the element to look for text
 * @param {Object} options - Additional options
 * @param {string|function} options.after - event name to trigger or callback function
 */
function fillable(selector, options) {
  if (typeOf(options.after) === 'string') {
    const AVAILABLE_EVENTS = ['focut-out', 'other-event', 'one-more-event'];

    if (!AVAILABLE_EVENTS.includes(options.after)) {
      throw EmberError('Invalid after event');
    }
  }
  // ... some code
}
export default PageObject.create({
  email: fillable('[data-test-login-email] input', { after: 'focut-out' }),
  password: fillable('[data-test-login-password] input', { after: 'focut-out' }),
});
@san650
Copy link
Owner

san650 commented Apr 30, 2019

I think having an option like that could be really useful, do you want to try to implement it?

That being said, I think It would be interesting to have an easy way to extend default helpers, I think we can do something like this now (pseudo code, not working I think):

function customFillable(selector, userOptions = {}) {
  const descriptor = fillable(selector, userOptions);

  return {
    isDescriptor: true,

    get(key) {
      const fn = descriptor.get.call(this, key);

      return function() {
        fn.call(this, ...arguments).then(() => blurPassword(selector));

        return this;
      }
    }
  }
}

const page = create({
  password: customFillable('.password')
});

just my 2¢

@artemgurzhii
Copy link
Contributor Author

@san650 Sure, I'll try to implement during this weekend

@ro0gr
Copy link
Collaborator

ro0gr commented Oct 4, 2019

Thanks for opening the issue and sorry for being so late here! 😅

I'm afraid a new API like action hooks can lead us a bit further from simlicity. As suggested by @san650, in his previous comment, there is an existing public way(though a bit verbose) to extend existing actions.

What is the benefit of the new concurrent API?

@ro0gr
Copy link
Collaborator

ro0gr commented Oct 5, 2019

Using master, today it should be possible to do something quite close to what you are asking for:

import { fillable } from 'ember-cli-page-object';
import { run } from 'ember-cli-page-object/-private/action';

function myFillable(selector, query = {}) {
  const descriptor = fillable(selector, query);

  return {
    isDescriptor: true,

    get(key) {
      const fillIn = descriptor.get.call(this, key);

      return function(contentOrClue, content) {
        let chain = run(this, function brake() {  
          return new Promise((r) => {
            return setTimeout(r, 100);
          })
        })

        // built-in fillable also uses `run` under the hood 
        chain = fillIn.call(chain, contentOrClue, content);

        return run(chain, () => {
          // `fillable` has some custom search logic for the `clue` arg,
          // so sometimes we may blur invalid DOM element  
          const el = findElementWithAssert(this, selector, query).get(0);

          return myBlur(el);
        });
      }
    }
  };
}

it still uses run, which has been added to the master just recently and is private.

@artemgurzhii artemgurzhii changed the title Feature: options after Feature: before and after hooks Oct 6, 2019
@artemgurzhii
Copy link
Contributor Author

@ro0gr yes, but from my point of view - we need to decide whether this feature brings enough use cases to be merged into master, I would rather use an addon realization of this feature than any other or custom. Also, it does not look for me like a step awayf from the simplicity, as fillable and other methods still can take options object as the last parameter, and adding 2 more keys is not a big deal.

If we still consider this as an overhead we can expose something like hookable which will be a HOF on top of the fillable,clickable,etc...

Example:

import { create, fillable as fill, hookable } from 'ember-cli-page-object';

const fillable = hookable(fill);

const page = create({
  fillInName: fillable('input', { after: 'blur' }),
});

@ro0gr What's your thoughts on this?

@ro0gr
Copy link
Collaborator

ro0gr commented Oct 6, 2019

@artemgurzhii I think the feature can have sense for some apps.
However, if you need to enable fillIn + blur for your fillable, I think someone would want this behavior enabled by default for the whole application, w/o a need to always specify this implementation detail({ after: 'blur' }) via extra options.

For me it means we just need to provide a good public way to define user actions, which obviously is a missing feature currently. And if we have that custom user action, I can see no features hooks would bring to us.

@ro0gr
Copy link
Collaborator

ro0gr commented Oct 6, 2019

Just realized, I might have missed your point.

fillable and other methods still can take options object as the last parameter, and adding 2 more keys is not a big deal.

Do you mean adding before and after options to the fillable and maybe to the others actions without introducing action hooks as a public API?

If talk specifically about the blur option for the fillable only..
I believe the blur is triggered by browser once another element is focused, so in most cases you should not care about trigerring it manually.
Also if we encode smth like after=blur in the page object itself, then we don't have a way to disable this behavior in the test. What if I want to test an element state right after fiiliing it, when its still focused? I believe it's often better to leave this decision for the test.

@artemgurzhii
Copy link
Contributor Author

I believe the blur is triggered by browser once another element is focused, so in most cases you should not care about trigerring it manually.

I think so too, but I remember that I had a use case to test that input had shown error message after blur, so here are two implementations of this(the first one is without hooks and the second one with them)

First:

page.writeInput1('text 1').writeInput2('text 2');

assert.ok(page.input1HasErrorClass, 'input 1 has error class');

Second:

page.writeInput1('text 1', { after: 'blur' })

assert.ok(page.input1HasErrorClass, 'input 1 has error class');

Using the first example does not actually work as expected, the blur effect is a side effect from writing text and I prefer not to rely on it + it's not easy to understand when you read a code like this what does it actually doing.

And to bring more context on the feature which I'm proposing - here are a few examples of the new API:

const afterFunction = () => { /*...*/ };

const page = create({
  login: fillable('input.login', { before: 'focus-in' }),
  password: fillable('input.password', { after: 'focus-out' }),
  submit: clickable('button.submit', { after: afterFunction })
});

Use cases I was originally thinking off was accepting a string as an argument where string is a valid event name, but for now, I think that we should allow also passing a function as there might be use cases for that too.

P.S. As this feature brings a lot of discussions - I would like to hear other people's thoughts on this. I think that addon should give an ability to users to use hooks, but should it be:

  • Documentation on how to create them?
  • Built-in hooks as I originally proposed?
  • Expose hookable HOF?
  • Other options?

@ro0gr
Copy link
Collaborator

ro0gr commented Oct 6, 2019

Using the first example does not actually work as expected, the blur effect is a side effect from writing text and I prefer not to rely on it

Agree, in my opinion, in this case test should explicitly declare an intention.

Other options?

Yes.

You can describe your inputs as a page object components:

const page = create({
  login: {
    scope: 'input.login',
    hasError: hasClass('has-error')
  },

  password: {
    scope: 'input.password',
    hasError: hasClass('has-error')
  },

  submit: clickable('button.submit')
});

In this case, the test can be re-written as follows:

test('validates on blur', async function(assert) {
  await page.login.fillIn('text 1')
    .blur();

  assert.ok(page.login.hasError, 'login field has error');
})

Each component is supplied with default attributes. That allows us to use blur w/o a need to declare it.

This is just my way of doing things and I'd recommend to take it in consideration at least. Hope it makes sense to you!


UPD: In addition to an ability to use blur for free, you can also go even further, and extract an input definition in order to keep input specific stuff in a single definition:

// I can be re-used in anywhere object
const Input = (scope) => {
  return {
    scope,
    hasError: hasClass('has-error'),
    // lot's of different input specific stuff...
  }
}

That would drastically reduce amount of boilerplate in your page objects:

const page = create({
  login: Input('input.login'),

  password: Input('input.password'),

  submit: clickable('button.submit')
});

@albuquerquecesar
Copy link

As @ro0gr suggested, you could define your inputs as page object components and create a helper for doing it.

const inputHelper = (scope) => ({
  scope,
  _fillIn: fillable(),
  fillIn(text) {
    return this.focus()._fillIn(text).blur();
  }
});

.....

const page = create({
loginInput: inputHelper(input.password'),
password: ...
});

// in test

page.loginInput.fillIn('my login');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants