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

v6.0.1 label tagName: '' update is a breaking change #129

Closed
raido opened this issue Jan 15, 2020 · 7 comments
Closed

v6.0.1 label tagName: '' update is a breaking change #129

raido opened this issue Jan 15, 2020 · 7 comments

Comments

@raido
Copy link

raido commented Jan 15, 2020

Recent v6.0.1 which converted label component to tagless component is kind of a breaking change.

See my comment on the commit:

0b9e39e#commitcomment-36809108

@knownasilya
Copy link
Owner

Give v6.0.2 a try.

@raido
Copy link
Author

raido commented Jan 17, 2020

The change overall is nice but it turned out one other implicit behaviour had changed.

Before v6.0.1 label elements were always rendered even if contents itself were empty:

c1ad3ba#diff-6b8cfae49a619d5b92f6b26f55cc03cfL1

After v6.0.1 things are wrapped in if this.show - which is the actual reason why tests in my app begin to fail with upgrading.

Because before v6.0.1 label elements were always rendered and those elements have actions on them, so simulating click in acceptance tests worked fine. Now it does not.

Only way I can make my tests work again is to use something like:

// template
<XToggle data-test-checkbox="my-stuff" .../> 

// in tests
await click('[data-test-checkbox="my-stuff"] .x-toggle-btn');

Switch component does not do any ...attributes spreading either: https://github.com/knownasilya/ember-toggle/blob/master/addon/components/x-toggle-switch/template.hbs#L15

@raido
Copy link
Author

raido commented Jan 17, 2020

Better "workaround" for my situation is actually to begin adding specific name on elements:

<XToggle @name="my-button">

await click('input[name="my-button"]');

And avoid using data-test attributes I guess?

For reference: https://github.com/knownasilya/ember-toggle/blob/master/addon/components/x-toggle-switch/template.hbs#L7

@knownasilya
Copy link
Owner

Before v6.0.1 label elements were always rendered even if contents itself were empty:

This was the case, but I think was the wrong behavior.

Switch component does not do any ...attributes spreading either: https://github.com/knownasilya/ember-toggle/blob/master/addon/components/x-toggle-switch/template.hbs#L15

This component is not tagless, so Ember applies the attributes automatically.

Better "workaround" for my situation is actually to begin adding specific name on elements:
<XToggle @name="my-button">
await click('input[name="my-button"]');

Might be worth documenting a basic test setup.

@raido
Copy link
Author

raido commented Jan 17, 2020

This component is not tagless, so Ember applies the attributes automatically.

This is correct but since none of the actions are bound to the outer switch span element but instead on input/label elements directly it's no use.

Triggering click on outer span has no effect.

@knownasilya
Copy link
Owner

Yeah, understood, but for accessibility the span should not be clickable. On another note, if we added ...attributes to the label since that's the visible clickable node, anything added will also be added to the base element, which is probably not what we want.

The real solution is to convert to Glimmer Components and set attributes only on the label. I'll probably be tackling that sometime in the next month.

@raido
Copy link
Author

raido commented Jan 17, 2020

Yes, that sounds like a proper solution.

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

2 participants