-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add Multiselect test helpers #254
Conversation
🦋 Changeset detectedLatest commit: 932ad6c The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Preview URLsEnv: preview |
16f42b0
to
05dd96d
Compare
onClick=(fn this.removeSelection index) | ||
onMouseDown=this.handleRemoveMouseDown | ||
isVisible=this.isEnabled | ||
<div data-multiselect> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No changes other than I wrapped everything in <div data-multiselect>
. This gives test helpers a parent (closest
) to select, which lets them select various children.
@@ -96,12 +96,12 @@ export default class ToucanCoreAutocompleteOptionComponent extends Component<Tou | |||
<template> | |||
{{! template-lint-disable require-presentational-children }} | |||
<li | |||
aria-current={{if @isActive "true" "false"}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaces data-active
.
@@ -97,12 +97,12 @@ export default class ToucanCoreMultiselectControlComponent extends Component<Tou | |||
{{! template-lint-disable require-presentational-children }} | |||
{{#let (uniqueId) as |id|}} | |||
<li | |||
aria-current={{if @isActive "true" "false"}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above screenshot and comment.
get active() { | ||
return this.element | ||
?.closest('[data-autocomplete]') | ||
?.querySelector('[aria-current="true"]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We spread attributes
on the <input />
, which seems right to me. However, it means we need to first get ahold of the parent element ([data-autocomplete]
) before we can select children.
@@ -9,7 +9,7 @@ module('Integration | Component | Fields | Autocomplete', function (hooks) { | |||
|
|||
test('it renders', async function (assert) { | |||
await render(<template> | |||
<Autocomplete @noResultsText="No results" @label="Label" data-autocomplete /> | |||
<Autocomplete @noResultsText="No results" @label="Label" data-input /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to data-input
throughout so it's clearer that it's the <input />
and not the component's wrapping element.
@@ -18,7 +18,7 @@ const ToucanCoreMultiselectChipComponent: TemplateOnlyComponent<ToucanCoreMultis | |||
<div | |||
class="min-h-6 bg-normal-idle flex items-center gap-x-2.5 rounded-sm px-2 py-1" | |||
data-index={{@index}} | |||
data-multiselect-selected-option | |||
data-multiselect-chip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Designers love their eats, huh? Chips, Snackbars, Toast, etc.)
05dd96d
to
0b8bd57
Compare
0b8bd57
to
9f21170
Compare
9f21170
to
5cc4679
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Some formatting changes (whitespace mostly)
- Some additional
dom.exists()
assertions - Swapped out selector strings for test helpers
5cc4679
to
932ad6c
Compare
Pushed an amended commit to resolve a merge conflict. |
932ad6c
to
532ab89
Compare
🚀 Description
🔬 How to Test
We're good if the build is green.