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

[TextField] Replace autocomplete off with nope #4053

Merged
merged 6 commits into from
Mar 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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');
Copy link
Member

Choose a reason for hiding this comment

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

Unblocked by Shopify/quilt#1764


(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