Skip to content

Commit

Permalink
[TextField] Replace autocomplete off with nope (#4053)
Browse files Browse the repository at this point in the history
* [TextField] Replace autocomplete off with nope

* Add to UNRELEASED

* Update the accessibility check to use the shared package

* Adding package and bumping node version

* Bump node version in workflows

* Fix iframe path

Co-authored-by: Alex Page <[email protected]>
  • Loading branch information
Dominic McPhee and Alex Page authored Mar 12, 2021
1 parent 433525e commit a4f6710
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 117 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:

- uses: actions/setup-node@v2
with:
node-version: '10.15.0'
node-version: '10.24.0'

- name: Get yarn cache directory path
id: yarn-cache-dir-path
Expand Down Expand Up @@ -45,7 +45,7 @@ jobs:

- uses: actions/setup-node@v2
with:
node-version: '10.15.0'
node-version: '10.24.0'

- name: Get yarn cache directory path
id: yarn-cache-dir-path
Expand Down Expand Up @@ -73,7 +73,7 @@ jobs:

- uses: actions/setup-node@v2
with:
node-version: '10.15.0'
node-version: '10.24.0'

- name: Get yarn cache directory path
id: yarn-cache-dir-path
Expand Down Expand Up @@ -104,7 +104,7 @@ jobs:

- uses: actions/setup-node@v2
with:
node-version: '10.15.0'
node-version: '10.24.0'

- name: Get yarn cache directory path
id: yarn-cache-dir-path
Expand Down
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v10.15.0
v10.24.0
1 change: 1 addition & 0 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
- Ensured `@charset` declaration is the first thing in our styles.css file ([#4019](https://github.com/Shopify/polaris-react/pull/4019))
- Fix `Modal.Section` divider color to match header and footer divider ([#4021](https://github.com/Shopify/polaris-react/pull/4021))
- Remove focus ring on click for ActionList ([#4034](https://github.com/Shopify/polaris-react/pull/4034))
- Updated `<TextField>` to use `autocomplete=nope` instead of `autocomplete=off` ([#4053](https://github.com/Shopify/polaris-react/pull/4053))

### Documentation

Expand Down
2 changes: 1 addition & 1 deletion dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: polaris-react
up:
- node:
yarn: v1.13.0
version: v10.15.0 # to be kept in sync with .nvmrc and ci.yml
version: v10.24.0 # to be kept in sync with .nvmrc and ci.yml
- git_hooks:
pre-commit: pre-commit

Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
"@shopify/sewing-kit": "^0.140.0",
"@shopify/shrink-ray": "^2.3.1",
"@shopify/splash": "^0.0.8",
"@shopify/storybook-a11y-test": "^0.0.1",
"@storybook/addon-a11y": "^6.1.11",
"@storybook/addon-console": "^1.2.1",
"@storybook/addon-contexts": "^5.3.19",
Expand Down Expand Up @@ -143,10 +144,8 @@
"node-sass": "^4.12.0",
"npm-run-all": "^4.1.5",
"object-hash": "^1.3.1",
"p-map": "^4.0.0",
"postcss": "^7.0.18",
"postcss-modules": "^3.1.0",
"puppeteer": "^1.20.0",
"react": "^16.9.0",
"react-dom": "^16.9.0",
"react-is": "^16.9.0",
Expand Down
118 changes: 18 additions & 100 deletions scripts/accessibility-check.js
Original file line number Diff line number Diff line change
@@ -1,107 +1,25 @@
/* eslint-disable no-console */
const os = require('os');
const path = require('path');

const puppeteer = require('puppeteer');
const pMap = require('p-map');
const chalk = require('chalk');

// eslint-disable-next-line node/no-path-concat
const iframePath = `file://${__dirname}/../build/storybook/static/iframe.html`;
const concurrentCount = os.cpus().length;
const skippedStoryIds = ['playground-playground', 'all-examples'];

const getUrls = async (browser) => {
// Get the URLS from storybook
const page = await browser.newPage();
await page.goto(iframePath);
const storyIds = await page.evaluate(() =>
window.__STORYBOOK_CLIENT_API__.raw().map((story) => story.id),
);
await page.close();

const urls = storyIds.reduce((memo, storyId) => {
// If it is a story id that is not in excludedStoryIds
if (skippedStoryIds.every((id) => !storyId.includes(id))) {
const url = `${iframePath}?id=${storyId}`;
memo.push(
url,
// Dark mode has lots of errors. It is still very WIP so ignore for now
// `${url}&contexts=Color%20scheme%3DDark%20Mode`,
);
}

return memo;
}, []);

return urls;
};

const formatFailure = (fail) => {
return ` - ${fail.failureSummary.split('\n ')[1]}\n ${fail.html}`;
};

const formatMessage = (id, violations) => {
const url = chalk.underline.blue(
`http://localhost:6006/iframe.html?id=all-components-${id}`,
);
return violations
.map((fail) => {
const message = fail.nodes
.map((error) => formatFailure(error))
.join('\n');
return `${chalk.red(fail.impact)} => ${url}\n${message}`;
})
.join('\n');
};
const {storybookA11yTest} = require('@shopify/storybook-a11y-test');

(async () => {
try {
// Open a browser
console.log(chalk.bold(`🌐 Opening ${concurrentCount} tabs in chromium`));
const browser = await puppeteer.launch();

// Get the test ids from storybook
const testIds = await getUrls(browser);

console.log(chalk.bold(`🧪 Testing ${testIds.length} urls with axe`));

// Test the pages with axe
const testPage = async (url) => {
const id = url.replace(`${iframePath}?id=all-components-`, '');
console.log(` - ${id}`);

try {
const page = await browser.newPage();
await page.goto(url);
const result = await page.evaluate(() =>
window.axe.run(document.getElementById('root'), {}),
);
await page.close();

if (result.violations.length) {
return formatMessage(id, result.violations);
}
} catch (error) {
return `please retry => ${id}:\n - ${error.message}`;
}
};

const results = await pMap(testIds, testPage, {
concurrency: concurrentCount,
});
await browser.close();

// Format the results and log them out
const messages = results.filter((x) => x);
if (messages.length) {
console.error(chalk.bold(`‼️ Test failures found`));
console.log(messages.join('\n'));
process.exit(1);
} else {
console.log(chalk.bold('🧚‍♀️ Accessibility is all g'));
}
} catch (error) {
console.error(error.message);
const options = {
iframePath: path.join(
'file://',
__dirname,
'../build/storybook/static/iframe.html',
),
skippedStoryIds: ['playground-playground', 'all-examples'],
};

const results = await storybookA11yTest(options);

if (results.length) {
console.error(`‼️ Test failures found`);
console.log(results.join('\n'));
process.exit(1);
} else {
console.log('🧚‍♀️ Accessibility is all g');
}
})();
4 changes: 3 additions & 1 deletion src/components/TextField/TextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,9 @@ function normalizeAutoComplete(autoComplete?: boolean | string) {
if (autoComplete === true) {
return 'on';
} else if (autoComplete === false) {
return 'off';
return 'nope';
} else if (autoComplete === 'off') {
return 'nope';
} else {
return autoComplete;
}
Expand Down
13 changes: 10 additions & 3 deletions src/components/TextField/tests/TextField.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,21 @@ describe('<TextField />', () => {
expect(textField.find('input').prop('autoComplete')).toBeUndefined();
});

it('sets autoComplete to "off" when false', () => {
it('sets autoComplete to "nope" when false', () => {
const textField = mountWithAppProvider(
<TextField label="TextField" autoComplete={false} onChange={noop} />,
);
expect(textField.find('input').prop('autoComplete')).toBe('off');
expect(textField.find('input').prop('autoComplete')).toBe('nope');
});

it('sets autoComplete to "on" when false', () => {
it('sets autoComplete to "nope" when "off"', () => {
const textField = mountWithAppProvider(
<TextField label="TextField" autoComplete="off" onChange={noop} />,
);
expect(textField.find('input').prop('autoComplete')).toBe('nope');
});

it('sets autoComplete to "on" when true', () => {
const textField = mountWithAppProvider(
<TextField label="TextField" autoComplete onChange={noop} />,
);
Expand Down
Loading

0 comments on commit a4f6710

Please sign in to comment.