Bump ember-test-selectors to v6 with fixed tests #937
Conversation
DanMonroe
force-pushed
the
monroe/et4-test-selectors
branch
from
be406a4 to
33237a2
Compare
To have the data-test attributes removed in a production build, tie into the setupForTest helper.
DanMonroe
force-pushed
the
monroe/et4-test-selectors
branch
from
b020b2b to
8386120
Compare
Due to an issue with ember-angle-bracket-invocation-polyfill ember-polyfills/ember-angle-bracket-invocation-polyfill#69 where data attributes are not applied properly to component with existing attributeBindings. Used class names to select the appropriate table instead.
| @@ -6,22 +6,23 @@ | |||
| data-test-select-row-container | |||
| > | |||
| {{-ember-table-private/simple-checkbox | |||
There was a problem hiding this comment.
<EmberTablePrivateSimpleCheckbox
There was a problem hiding this comment.
Can't convert to angle brackets because of the 2.18 expected an implementation for DynamicArg issue
| @@ -218,7 +218,8 @@ export default Component.extend({ | |||
| */ | |||
| onResize: null, | |||
|
|
|||
| 'data-test-row-count': readOnly('wrappedRows.length'), | |||
| attributeBindings: ['dataTestRowCount:data-test-row-count'], | |||
| dataTestRowCount: readOnly('wrappedRows.length'), | |||
There was a problem hiding this comment.
The implementations here (thead and in tfoot) for row count binding should match the implementation for tbody?
There was a problem hiding this comment.
Ok, I've left some comments for followup, but lets merge this and fix-forward any concerns.
Thank you @DanMonroe for your diligent efforts here, and patience with my feedback 🎉
| @@ -5,23 +5,23 @@ | |||
| class="et-toggle-select {{unless this.shouldShowCheckbox 'et-speech-only'}}" | |||
| data-test-select-row-container | |||
| > | |||
| {{-ember-table-private/simple-checkbox | |||
| data-test-select-row=true | |||
| {{ember-table-simple-checkbox | |||
There was a problem hiding this comment.
The intent of dropping the / in the invocation (by moving the component to the top level) was to allow us to use angle bracket invocation in 3.4 without the polyfill. If angle bracket invocation is not being used here, I don't believe the renaming is strictly necessary.
| @@ -218,11 +223,16 @@ export default Component.extend({ | |||
| */ | |||
| onResize: null, | |||
|
|
|||
| 'data-test-row-count': readOnly('wrappedRows.length'), | |||
| attributeBindings: ['wrappedRowsCount:data-test-row-count'], | |||
| wrappedRowsCount: and('isTesting', 'wrappedRows.length'), | |||
There was a problem hiding this comment.
I don't believe this actually stops wrappedRows.length from being a dependency in the chain, which is the goal of a testing mode.
This PR has several code changes surrounding attribute bindings.
Once #936 is approved and merged, I'll rebase this PR.