Skip to content

Commit

Permalink
Fix icons/logos with static SVG IDs to use generated IDs (#5204)
Browse files Browse the repository at this point in the history
* Remove unnecessary ID from stopSlash icon

* [scripts setup] Add check for static SVG IDs

- DRY out toString() to one var
- switch from indexOf to the slightly more readable `.includes()` API

* Set up htmlIdGenerator util for SVGs with IDs

- Use different template with htmlIdGenerator import/setup + non-implicit return
- tweak template literal indenting
- DRY out fileName for use as generator prefix

* Replace the static SVG id attributes with dynamic JSX

NOTE: I'm avoiding doing this in the `ast` process for two main reasons:

1. I'm a monster who think regexes are neat

2. the ${jsx} in the ast template is an actual tree of components and it seems much more tedious to iterate through each one & examine its attributes (of which the url() attr can have multiple - e.g. fill or mask), rather than use a basic regex capture group

* Clean up ID prefixes

- in their current state, all icons (except for dropwizard) will automatically truncate their ID names to 'a', 'b', etc. when cleanupIDs is set to true

* Run yarn compile-icons

moment of truth!

* Update snapshots

* Add changelog entry
  • Loading branch information
Constance authored Sep 23, 2021
1 parent fb31721 commit 0da641c
Show file tree
Hide file tree
Showing 14 changed files with 803 additions and 735 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## [`master`](https://github.com/elastic/eui/tree/master)

No public interface changes since `38.1.0`.
**Bug fixes**

- Fixed logo icons with static SVG IDs causing accessibility errors when multiples of the same logo were present ([#5204](https://github.com/elastic/eui/pull/5204))

## [`38.1.0`](https://github.com/elastic/eui/tree/v38.1.0)

Expand Down
38 changes: 31 additions & 7 deletions scripts/compile-icons.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,24 @@ function pascalCase(x) {
const iconFiles = glob.sync('**/*.svg', { cwd: iconsDir, realpath: true });

iconFiles.forEach(async (filePath) => {
const fileName = path.basename(filePath, '.svg');
const svgSource = fs.readFileSync(filePath);
const svgString = svgSource.toString();

try {
const viewBoxPosition = svgSource.toString().indexOf('viewBox');
if (viewBoxPosition === -1) {
if (!svgString.includes('viewBox')) {
throw new Error(`${filePath} is missing a 'viewBox' attribute`);
}

const jsxSource = await svgr(
const hasIds = svgString.includes('id="');

let jsxSource = await svgr(
svgSource,
{
plugins: ['@svgr/plugin-svgo', '@svgr/plugin-jsx'],
svgoConfig: {
plugins: [
{ cleanupIDs: false },
{ cleanupIDs: true },
{ prefixIds: false },
{ removeViewBox: false },
],
Expand All @@ -43,17 +46,38 @@ iconFiles.forEach(async (filePath) => {
{ template },
opts,
{ imports, componentName, props, jsx }
) => template.ast`
) =>
hasIds
? template.ast`
${imports}
import { htmlIdGenerator } from '../../../services';
const ${componentName} = (${props}) => {
const generateId = htmlIdGenerator('${fileName}');
return (
${jsx}
);
};
export const icon = ${componentName};
`
: template.ast`
${imports}
const ${componentName} = (${props}) => ${jsx}
export const icon = ${componentName};
`,
`,
},
{
componentName: `EuiIcon${pascalCase(path.basename(filePath, '.svg'))}`,
componentName: `EuiIcon${pascalCase(fileName)}`,
}
);

// Replace static SVGs IDs with dynamic JSX that uses the htmlIdGenerator
if (hasIds) {
jsxSource = jsxSource
.replace(/id="(\S+)"/gi, "id={generateId('$1')}")
.replace(/"url\(#(\S+)\)"/gi, "{`url(#${generateId('$1')})`}")
.replace(/xlinkHref="#(\S+)"/gi, "xlinkHref={`#${generateId('$1')}`}");
}

const outputFilePath = filePath.replace(/\.svg$/, '.js');
const comment = '// THIS IS A GENERATED FILE. DO NOT MODIFY MANUALLY\n\n';
fs.writeFileSync(outputFilePath, comment + jsxSource);
Expand Down
Loading

0 comments on commit 0da641c

Please sign in to comment.