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

Update test run page & replace renderer's iframe #315

Merged
merged 51 commits into from
Sep 7, 2021

Conversation

howard-e
Copy link
Contributor

@howard-e howard-e commented Aug 16, 2021

This aims to update the Test Run page's functionality to bring it up to date with our current backend implementation.

Before merge checklist:

Recommended Setup Script

if [[ "$OSTYPE" == "darwin"* ]]; then
    createdb # run this if the PostgreSQL installation is freshly installed
    yarn db-init:dev;
else
    sudo -u postgres yarn db-init:dev;
fi;

yarn sequelize db:migrate;
yarn sequelize db:seed:all;
yarn workspace server db-import-tests:dev;

The major feature addition here is that the iframe which would previously hold the test content is now built and ran locally and depends on w3c/aria-at#466.

Known Issues

  • Focus is lost after closing Test Window - to be addressed

howard-e added 18 commits August 2, 2021 17:22
# Conflicts:
#	client/components/App/App.jsx
#	client/components/StatusBar/index.jsx
#	client/components/TestQueue/index.jsx
#	client/components/TestQueue/queries.js
#	client/components/TestQueueRun/TestQueueRun.css
#	client/components/TestQueueRun/index.jsx
#	client/components/TestRun/index.jsx
#	server/graphql-schema.js
#	server/resolvers/TestPlanReport/conflictsResolver.js
#	server/resolvers/TestPlanRun/index.js
#	server/resolvers/TestPlanRun/testResultCountResolver.js
@sinabahram
Copy link

We should pick a style for buttons and then stick with it e.g. title case or just initial caps?

I'm confused about "My Issue is not in this list". I think the only two logical possibilities are "My Issue Is Not In This List" or "My issue is not in this list". I don't understand the caps on both "My" and "Issue".

Title case for buttons seems very logical IMHO, so that would be my vote.

Copy link
Contributor

@alflennik alflennik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is aweeesome, can't wait to merge it.

I'm curious about the aria-at-harness.mjs, aria-at-test-io-format.mjs and the other files in this resources folder - I think I sort of recognize them from the ARIA-AT repo? Are they copied here? Is that permanent? Or are they meant to stay?

No real notes besides that question, I should be able to approve pretty soon after you feel like the blockers are clear.

@howard-e
Copy link
Contributor Author

howard-e commented Aug 17, 2021

@sinabahram
We should pick a style for buttons and then stick with it e.g. title case or just initial caps?

Agreed. Title Case v Sentence case is a decision to be made.

@alflennik
I'm curious about the aria-at-harness.mjs, aria-at-test-io-format.mjs and the other files in this resources folder - I think I sort of recognize them from the ARIA-AT repo? Are they copied here? Is that permanent? Or are they meant to stay?

This isn't permanent. This was mainly for ease of debugging purposes.

@howard-e howard-e force-pushed the new-test-run-frontend branch from 2bb0d41 to c3345b6 Compare August 24, 2021 18:25
@howard-e
Copy link
Contributor Author

howard-e commented Aug 31, 2021

@alflennik @jesdaigle, this is now ready for your review.

Copy link
Contributor

@alflennik alflennik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having tested this thoroughly on the sandbox, staging, etc., and using it as the base for my current branch, I can safely say this is ready for merge, and it's fantastic stuff!

Copy link
Contributor

@jesdaigle jesdaigle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is so pretty. 🤩

@howard-e howard-e merged commit c8cc93a into develop Sep 7, 2021
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

Successfully merging this pull request may close these issues.

5 participants