This repository was archived by the owner on May 24, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 166
[terra-icon] accessibility upgrade #3598
Merged
Merged
Changes from 41 commits
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
8875918
added isdecorative logic
acf4172
added svg source prop
634bca5
converted tag to img tag
7131b51
updated props and logic for terra-icon
e4bc1fc
almost working wdio tests
946f51e
updated wdio snapshots
ab22f7e
updated wdio snapshots
8d34365
initial commit
300a5fb
updated IconBase to be meaningful
faee111
made prop required
0e779de
added title to examples
62b70c0
updated icon
f57f200
updated icon
9f687f4
updated docs and iconbase
04d7021
Merge branch 'issue-6242' of https://github.com/cerner/terra-core int…
jeremyfuksa 67f695e
updated title prop to a11yLabel
841dce8
updated icon all tests
f58c2d8
formatted all icons test
3248c89
updated props in documentation
f3c8e48
updated changelog
5db4499
updated test labels
91340a1
generated decorative icons, untested
73a54b2
Merge branch 'issue-6242' of https://github.com/cerner/terra-core int…
e0100aa
documenting decorative icon generation
4b66ada
whoops... :)
2013fc4
fixed prop typo
b399fbe
updated scripts
52f9355
created decorative icons
4277119
updated decorative icons
598d9d8
updated decorative icons
c689895
finished icons
7324ffc
finished icon tests
d1f74e5
added documentation
3b8ad03
Merge branch 'issue-6242' of https://github.com/cerner/terra-core int…
jeremyfuksa 73c4ca4
updated docs
6eb54de
updated tests
4f16e98
undoed merge conflicts
bea1d0f
undoed merge conflicts
2ad806b
updated generation scripts
14c694b
updated icons
6f33283
restored snapshots
8175669
Update packages/terra-icon/scripts/src/README.md
sdadn 6e36b79
updated scripts
bdda562
updated scripts
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
94 changes: 47 additions & 47 deletions
94
packages/terra-core-docs/src/terra-dev-site/doc/icon/example/IconStatic.jsx
Large diffs are not rendered by default.
Oops, something went wrong.
456 changes: 228 additions & 228 deletions
456
packages/terra-core-docs/src/terra-dev-site/doc/icon/example/IconThemeable.jsx
Large diffs are not rendered by default.
Oops, something went wrong.
19 changes: 19 additions & 0 deletions
19
packages/terra-core-docs/src/terra-dev-site/test/icon/IconAccessibleLabel.test.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import React from 'react'; | ||
import IconAlert from 'terra-icon/lib/icon/IconAlert'; | ||
import IconAlertDecorative from 'terra-icon/lib/icon/decorative/IconAlert'; | ||
|
||
const IconAccessibleLabel = () => ( | ||
<div> | ||
<h3>Meaningful icon</h3> | ||
<div> | ||
<IconAlert a11yLabel="alert icon"/> | ||
</div> | ||
|
||
<h3>Decorative icon</h3> | ||
<div> | ||
<IconAlertDecorative a11yLabel="alert icon"/> | ||
</div> | ||
</div> | ||
); | ||
|
||
export default IconAccessibleLabel; |
2 changes: 1 addition & 1 deletion
2
packages/terra-core-docs/src/terra-dev-site/test/icon/IconAll.module.scss
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
:local { | ||
.icon { | ||
height: 40px; | ||
margin: 5px; | ||
width: 40px; | ||
margin: 5px; | ||
} | ||
|
||
.icon-inverse { | ||
|
618 changes: 309 additions & 309 deletions
618
packages/terra-core-docs/src/terra-dev-site/test/icon/IconAll.test.jsx
Large diffs are not rendered by default.
Oops, something went wrong.
10 changes: 0 additions & 10 deletions
10
packages/terra-core-docs/src/terra-dev-site/test/icon/IconAriaLabel.test.jsx
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,20 @@ | ||
# Update all SVG icons | ||
## Step 1 - Migrate Cerner One Icons | ||
|
||
``` | ||
npm run migrate-cerner-one-icons | ||
``` | ||
* npm run compilescripts | ||
* npm run migrate-csv | ||
* npm run migrate-svg | ||
* npm run generate-icon | ||
* npm run generate-example | ||
npm run migrate-cerner-one-icons | ||
|
||
## Step 2 - Generate meaningful icons | ||
|
||
npm run compilescripts | ||
npm run migrate-csv | ||
npm run migrate-svg | ||
npm run generate-icon | ||
npm run generate-example | ||
|
||
## Step 3 - Generate decorateive icons | ||
sdadn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This part requires perl 5 to run. | ||
|
||
npm run create-decorative-dir | ||
npm run replace-decorative-baseclass | ||
npm run replace-decorative-displayname |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import classNames from 'classnames'; | ||
import classNamesBind from 'classnames/bind'; | ||
|
||
// eslint-disable-next-line import/no-unresolved, import/no-webpack-loader-syntax | ||
import styles from './Icon.module.scss'; | ||
|
||
const cx = classNamesBind.bind(styles); | ||
|
||
const propTypes = { | ||
/** | ||
* Should the svg mirror when dir="rtl". | ||
*/ | ||
isBidi: PropTypes.bool, | ||
/** | ||
* Should the SVG rotate. | ||
*/ | ||
isSpin: PropTypes.bool, | ||
/** | ||
* Child nodes. | ||
*/ | ||
children: PropTypes.node, | ||
/** | ||
* Height of SVG. | ||
*/ | ||
height: PropTypes.string, | ||
/** | ||
* Width of SVG. | ||
*/ | ||
width: PropTypes.string, | ||
/** | ||
* Focusable attribute. IE 10/11 are focusable without this attribute. | ||
*/ | ||
focusable: PropTypes.string, | ||
}; | ||
|
||
const defaultProps = { | ||
isBidi: false, | ||
isSpin: false, | ||
children: null, | ||
height: '1em', | ||
width: '1em', | ||
focusable: 'false', | ||
}; | ||
|
||
const IconBase = ({ | ||
isBidi, | ||
isSpin, | ||
children, | ||
height, | ||
width, | ||
title, | ||
focusable, | ||
...customProps | ||
}) => { | ||
const attributes = { ...customProps }; | ||
|
||
// append to existing classNames | ||
const classes = classNames( | ||
cx( | ||
'tui-Icon', | ||
'icon', | ||
{ 'is-bidi': isBidi }, | ||
{ 'is-spin': isSpin }, | ||
), | ||
attributes.className, | ||
); | ||
|
||
attributes.height = height; | ||
attributes.width = width; | ||
attributes.focusable = focusable; | ||
attributes.role = 'presentation'; | ||
|
||
return <svg {...attributes} className={classes}>{children}</svg>; | ||
}; | ||
|
||
IconBase.propTypes = propTypes; | ||
IconBase.defaultProps = defaultProps; | ||
|
||
export default IconBase; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A command could be added for each heading in this file with more than one command under it:
update-all-svg-icons
,generate-meaningful-icons
,generate-decorative-icons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated scripts here: 6e36b79
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dove a little deeper into this:
npm run migrate-cerner-one-icons
won't work now becausenpm generate-icon
isn't a script.npm run generate-all-icons
is supposed to be part ofnpm run migrate-cerner-one-icons
, wouldn't all steps in this document be completed in step 1?npm run generate-icon
isn't a thing now.npm run replace-decorative-import
is part of thegenerate-decorative-icons
command but isn't in the list of commands in this file to generate decorative iconsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh try pulling. I updated the readme and scripts.