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

terra-toolkit v5 #291

Merged
merged 36 commits into from May 31, 2019
Merged

terra-toolkit v5 #291

merged 36 commits into from May 31, 2019

Conversation

mjhenkes
Copy link
Contributor

Summary

These are the reverted changes re-applied. Watch this PR for the additional changes we will be making to toolkit.

@cerner/terra

@terra-bot
Copy link

Warnings
⚠️

❗ Big PR. Consider breaking this into smaller PRs if applicable

Generated by 🚫 dangerJS

const beAccessible = (...args) => {
accessibleItBlock(determineOptions.axeOptions(args));
const validatesAccessibility = (...args) => {
runAccessibilityTest(determineOptions.axeOptions(args));
Copy link
Contributor Author

@mjhenkes mjhenkes May 24, 2019

Choose a reason for hiding this comment

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

Instead of having both functions call runAccessibilityTest, you could have itIsAccessible call validatesAccessibility

const validatesAccessibility = (...args) => {
  const axeResults = global.browser.axe(determineOptions.axeOptions(args));
  global.expect(axeResults).to.be.accessible();
};

const itIsAccessible = (...args) => {
  global.it('is accessible', () => {
    runAccessibilityTest(...args);
  });
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Talked offline, we need to keep as is for people using Terra.validates.element such the validates.element ignores viewports configuration

@@ -17,13 +17,13 @@ function accessible() {
this.assert(
errors.length === 0,
`expected no accessibility violations but got:\n\t${errors[0]}`,
'expected accessibilty errors but received none',
'expected accessibility errors but received none',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

code spell checker is amazing

};

export default validateElement;
const validatesElement = (...args) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

you could DRY up these methods like you did for the accessibility functions above

});
};

const validatesScreenshot = (...args) => {
const {
name, selector, misMatchTolerance, viewports,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ignore viewports here

The default form factor is now 'huge' to correct inconsistent viewport sizing that had occurred when a test used the default viewport for a test run vs defining a huge viewport. This may require screenshot updates, but no code changes are necessary.

### TerraService
- The `viewportChangePause` option was removed the `Terra.should.matchScreenshot`.
Copy link
Contributor

Choose a reason for hiding this comment

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

"option was removed the" -> "option was removed from the"


For static sites, the original files will be served, you will be responsible for adding the locale to the static files.

For compiled sites, the ```defaultLocale```` environment variable will be passed to the webpack config indicating what locale the site should be compiled for. This will be done automatically for projects using terra-dev-site.
Copy link
Contributor

Choose a reason for hiding this comment

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

```defaultLocale```` -> ```defaultLocale```

```

### AxeService
The AxeService has been removed. The accessibility capabilities the Axe Service provided has been move to the Terra Service because the services could not run independently. The `axe` key can still be used for configuration and the test helper and axe chai assertion can be used. Please note, the `runOnly` option has been removed from Terra.should.beAccessible test helper and axe chai method and resetScroll has been enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

"has been move" -> "has been moved"

The nightwatch utility and peer dependencies have been removed in this toolkit release. Be sure to remove the `nightwatch` dev-dependency in your project, if it exists.

## NEXT: Upgrade to Toolkit v5
See Part 2 for more information on the step-by-step dependency and script changes for terra-toolkit v5.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could link to the URL part 2 will have when this is merged to master

@@ -0,0 +1,230 @@
# Terra Toolkit Upgrade Guide v5.0.0 - Part 2
Copy link
Contributor

Choose a reason for hiding this comment

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

File has "v5.1.0" in it's name

```

## NEXT: Webdriver Test Updates
See Part 3 for more information on Webdriver changes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Also could link to the URL part 3 will have when this is merged to master

```

### 2. Add a `start-prod` script
Add the following script to run `webpack-dev-server` with hot-reloading disabled to view the assets that are used during the test run.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be more specific about what "the assets that are used during the test run" means

package.json Outdated Show resolved Hide resolved
@mjhenkes mjhenkes merged commit 4698e4b into master May 31, 2019
@mjhenkes mjhenkes deleted the v5 branch May 31, 2019 14:32
@emilyrohrbough emilyrohrbough added this to In Review in Toolkit V5 via automation May 31, 2019
@emilyrohrbough emilyrohrbough moved this from In Review to Ready for Release in Toolkit V5 May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Toolkit V5
  
Ready for Release
Development

Successfully merging this pull request may close these issues.

None yet

6 participants