-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow test data toggle to be clicked #7479
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7479 +/- ##
==========================================
- Coverage 55.48% 55.21% -0.27%
==========================================
Files 672 672
Lines 26961 26961
Branches 2620 2620
==========================================
- Hits 14958 14887 -71
- Misses 11286 11354 +68
- Partials 717 720 +3
... and 15 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
test('ConditionSet has correct outputs when test data is enabled', async ({ page }) => { | ||
const exampleTelemetry = await createExampleTelemetryObject(page); | ||
|
||
await page.getByTitle('Show selected item in tree').click(); |
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.
this isn't possible to do with linting, but we cannot use .getByTitle since title's aren't compatible with screen readers or on mobile
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.
Hmm. We're doing this everywhere in our tests. Perhaps it needs a separate PR to change it in one go?
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.
Actually I've just done that in this PR.
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.
They just added a linting rule for us to get away from this
e2e/tests/functional/plugins/conditionSet/conditionSet.e2e.spec.js
Outdated
Show resolved
Hide resolved
@@ -136,6 +145,15 @@ export default { | |||
telemetryMetadataOptions: {} | |||
}; | |||
}, | |||
computed: { | |||
testDataClass() { |
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.
@charlesh88 can you check to see that this is how we want to do this disabled
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.
Closes #7424
Describe your changes:
Add the disabled class instead of the disabled attribute
Add aria labels for drop downs and inputs
All Submissions:
Author Checklist
type:
label? Note: this is not necessarily the same as the original issue.Reviewer Checklist