Allow collections to have a parm for options

[cah-brian-gantzler]

Most of the items allow for a options hash, for example, text as in

users: collection('table tr', {
    firstName: text('td', { at: 0 }),
    lastName: text('td', { at: 1 })
  })

The options hash is providing the { at: 1}. I had a set of divs inside a td and collection does not provide an options hash. The code I wrote was

rows: collection('.clinical-order-dispense table tbody tr', {
    dispenseDateTime: text('td', {at: 0}),
    serialNumber: text('td', {at: 1}),
    drugInformation: {
      scope: "td:nth-child(3)",
      drug: text('div', {at: 0}),
      lot: text('div', {at: 1}),
      expiration: text('div', {at: 2}),
      segment: text('div', {at: 3})
    },
    dispenseEye: text('td', {at: 3}),
    dispenseOverride: text('td', {at: 4})
  })

seems weird I can use { at: 2 } where needed except had to resort to td:nth-child(3)

If collection allowed for the option hash, I assume I could have written

rows: collection('.clinical-order-dispense table tbody tr', {
    dispenseDateTime: text('td', {at: 0}),
    serialNumber: text('td', {at: 1}),
    drugInformation: collection('td', { at: 2}, {
      drug: text('div', {at: 0}),
      lot: text('div', {at: 1}),
      expiration: text('div', {at: 2}),
      segment: text('div', {at: 3})
    },
    dispenseEye: text('td', {at: 3}),
    dispenseOverride: text('td', {at: 4})
  })

[ro0gr]

Hey, thank you for opening the ticket!

I think this is more related to components query API rather than collection API.

For instance, let's assume the following markup:

<section class="MyComponent">
  <div>1</div>
  <div>2</div>
  <div>3</div>
</div>

Here we have the same issue, so in order to reference children divs, we have to use nth-child:

{
  scope: '.MyComponent',
  column1: {
    scope: 'div:nth-child(1)'
  },
  column2: {
    scope: 'div:nth-child(2)'
  },
  column3: {
    scope: 'div:nth-child(3)'
  }
}

cause we don't have at api at component definition level. So if we really want to address this issue of anonymous positional elements selector, we should probably consider adding of at option to the component definition.

TBH, I'm not sure if we really have to improve positional query APIs(at), cause it can easily lead to fragile tests, for example when you just swap your columns, but everything keeps working properly from the functional point of view.

It might be not applicable for each project, but in order to avoid this issue, in my projects, I usually mark each column with it's own class or data-test attribute to avoid the need for selecting by position, so we have smth like:

{{#let "MyComponent" as |scope|}}
  <section class="{{scope}}">
    <div class="{{scope}}-c1">1</div>
    <div class="{{scope}}-c2">2</div>
    <div class="{{scope}}-c3">3</div>
  </div>
{{/let}}

const scope = .MyComponent';
{
  scope,
  column1: {
    scope: `${scope}-c1`
  },
  column2: {
    scope: `${scope}-c2`
  },
  column3: {
    scope: `${scope}-c3`
  }
}

which leads to a more stable page object, because we rely on semantic rather than position. Would it work for you? Is at still feels like a missing part in the component definition?

[cah-brian-gantzler]

Yea I can definitively see that the drug information is not really a collection, but was the closest I saw. I in fact did use a component with the at encoded as a nth-child and having AT at the component level would have been more correct. Assigning each inner div its own class would have removed the ATs on the 4 divs and Im fine with going that way.

The real crux of the matter is the at on the drugInformation, I suggested collection because thats were you are already teaching at, but agree an at on the component definition may be more accurate

[ro0gr]

I see what you mean, and I thiknk you are right. It seems a good idea to unify query API.

Though adding the second collection argument for query configuration may feel natural comparing to the rest of props like text(".class", { at: 1 }) it has its drawbacks, what if the collection item component does also have some query configuration?

  drugInformation: collection('td', { 
    at: 2,
    resetScope: false
  }, {
    resetScope: true,
    text: text('div', { at: 1 })
  });

should resetScope be merged and who wins? That can be agreed, but still feels harder to reason about than we want it to be.

If talking about adding the at support at the definition, alongside the scope, testContainer and resetScope, I'd say it would pollute the definition and increase a risk(slight though) to collide with the user defined properties.

I believe a good approach may be to support scope as a query object, like described here.

With that we can easily achieve an aligned API, so you example would be possible to express, like:

  rows: collection('.clinical-order-dispense table tbody tr', {
    dispenseDateTime: text({
     selector: 'td',
     at: 0
    }),
    serialNumber: text({
      selector: 'td', 
      at: 1
    }),
    drugInformation: {
      scope: {
        selector: "td",
        at: 2
      },
      drug: text({
        selector: 'div', 
        at: 0
      }),
      // ...
    }
  })

[cah-brian-gantzler]

What you have at the bottom is what I would have expected. You did combine selector and options, which makes scope work really well and the change to text() to combine them does make then all look and act the same. But the change to text() and other actions will be a large lift for everyone to upgrade. I would hope you would plan on supporting text(string, options), and the new text(options) for a gradual migration.

[cah-brian-gantzler]

Ok, so I found that the at already works with scope in a component. We dont actually have to add it, just document it. Also some weird behaviour with at when not in a collection.

Code I have in a previous comment, using the nth-child.

rows: collection('.clinical-order-dispense table tbody tr', {
    dispenseDateTime: text('td', {at: 0}),
    serialNumber: text('td', {at: 1}),
    drugInformation: {
      scope: "td:nth-child(3)",
      drug: text('div', {at: 0}),
      lot: text('div', {at: 1}),
      expiration: text('div', {at: 2}),
      segment: text('div', {at: 3})
    },

rewritten code using at with scope. This actually works right now

  rows: collection('.clinical-order-dispense table tbody tr', {
    dispenseDateTime: text('td', {at: 0}),
    serialNumber: text('td', {at: 1}),
    drugInformation: {
      scope: "td",
      at: 2,
      drug: text('div', {at: 1}),
      lot: text('div', {at: 2}),
      expiration: text('div', {at: 3}),
      segment: text('div', {at: 4})
    },

Things to note.

• nth-child was 3 not 2 (it must start at 1, not 0 as the at with a collection), you see the at with scope is back to 2
• The at for drug etc started at 0 when used with nth-child but start at 1 when at is used within a component.
• at actually uses :eq(x) not :nth-child, not sure if that somehow influences the at in items under a component starting at 1

[cah-brian-gantzler]

I do like the change to scope below though as its more consistent and can add more options in the future without a name conflict.

scope: {
        selector: "td",
        at: 2
      },

[ro0gr]

Great findings!

I was not aware at is a thing on a component.

I'm surprised by

The at for drug etc started at 0 when used with nth-child but start at 1 when at is used within a component

if feels like at should be stable despite it's declared in nth-child or at context 🤔

I would hope you would plan on supporting text(string, options)

yes, I can understand mirgration pain of such a breaking change and I don't intend to break it, at least in a foreseeable future.

[cah-brian-gantzler]

Should this issue be closed and another open to actually implement the changes to at?

[ro0gr]

yes, I think we can close it, cause sounds like it's possible to lookup component by at within a collection.

And it'd be nice to figure out at inconsistent behavior, you've mentioned above, in a dedicated bug ticket.