Removes <use> and <def> from svg icons#2162
Closed
mdefazio wants to merge 8 commits intoelastic:masterfrom
Closed
Conversation
Update defazio fork to 13.0
Upstream merge
cchaos
suggested changes
Jul 24, 2019
Contributor
cchaos
left a comment
There was a problem hiding this comment.
Just saw a couple lingering id attributes. But all the icons still look the same rendered.
You'll need to rebase with master as well.
CHANGELOG.md
Outdated
| ## [`master`](https://github.com/elastic/eui/tree/master) | ||
|
|
||
| - Added `partial` glyph to `EuiIcon` ([#2152](https://github.com/elastic/eui/pull/2152)) | ||
| - Removes <use> and <def> from svg icons ([#2162](https://github.com/elastic/eui/pull/2162)) |
Contributor
There was a problem hiding this comment.
We like to write our changelog entries in the past tense, so you can just change this to 'Removed'
| <g> | ||
| <use xlink:href="#boxes_vertical-a"/> | ||
| </g> | ||
| <path id="boxes_vertical-a" d="M7,1 L7,3 L9,3 L9,1 L7,1 Z M6,0 L10,0 L10,4 L6,4 L6,0 Z M6,6 L10,6 L10,10 L6,10 L6,6 Z M7,7 L7,9 L9,9 L9,7 L7,7 Z M6,12 L10,12 L10,16 L6,16 L6,12 Z M7,13 L7,15 L9,15 L9,13 L7,13 Z"/> |
| <g> | ||
| <use xlink:href="#brush-a"/> | ||
| </g> | ||
| <path id="brush-a" d="M11.9928058,8.16982791 C11.9928058,9.00087929 11.3203437,9.67717623 10.4940048,9.67717623 L5.4980016,9.67717623 C4.67166267,9.67717623 3.99920064,9.00087929 3.99920064,8.16982791 L3.99920064,6.66247959 L11.9928058,6.66247959 L11.9928058,8.16982791 Z M8.99520384,14.1680693 C8.99520384,14.6232885 8.54656275,14.9951011 7.9960032,14.9951011 C7.44544365,14.9951011 6.99680256,14.6232885 6.99680256,14.1680693 L6.99680256,10.6820751 L8.99520384,10.6820751 L8.99520384,14.1680693 Z M3.99920064,5.65758071 L3.99920064,1.00489888 L5.26119105,1.00489888 L5.26119105,5.65758071 L3.99920064,5.65758071 Z M6.26039169,5.65758071 L6.26039169,1.00489888 L7.50439648,1.00489888 L7.50439648,5.65758071 L6.26039169,5.65758071 Z M8.50359712,5.65758071 L8.50359712,1.00489888 L9.73860911,1.00489888 L9.73860911,5.65758071 L8.50359712,5.65758071 Z M10.7378098,5.65758071 L10.7378098,1.00489888 L11.9918066,1.00489888 L11.9918066,5.65758071 L10.7378098,5.65758071 Z M3.00799361,-1.8e-15 L3,8.16982791 C3,9.55457857 4.12110312,10.6820751 5.4980016,10.6820751 L5.99760192,10.6820751 L5.99760192,14.1680693 C5.99760192,15.1779927 6.89388489,16 7.9960032,16 C9.0981215,16 9.99440448,15.1779927 9.99440448,14.1680693 L9.99440448,10.6820751 L10.4940048,10.6820751 C11.8709033,10.6820751 12.9920064,9.55457857 12.9920064,8.16982791 L13,-1.8e-15 L3.00799361,-1.8e-15 Z"/> |
| <g> | ||
| <use xlink:href="#pencil-a"/> | ||
| </g> | ||
| <path id="pencil-a" d="M12.1480732,3.14807323 L11,2 L2,11 L2,14 L5,14 L14,5 L12.8560732,3.85607323 L4.854,11.854 C4.756,11.951 4.628,12 4.5,12 C4.372,12 4.244,11.951 4.146,11.854 C3.951,11.658 3.951,11.342 4.146,11.146 L12.1480732,3.14807323 Z M11,1 C11.256,1 11.512,1.098 11.707,1.293 L14.707,4.293 C15.098,4.683 15.098,5.317 14.707,5.707 L5.707,14.707 C5.52,14.895 5.265,15 5,15 L2,15 C1.448,15 1,14.552 1,14 L1,11 C1,10.735 1.105,10.48 1.293,10.293 L10.293,1.293 C10.488,1.098 10.744,1 11,1 Z M5,14 L2,14 L2,11 L5,14 Z" /> |
* Added `fullWidth` and `isInvalid` props to EuiFilePicker * Added a ‘large’ vs 'default' version * Setting the height statically so that it doesn't shift it's surrounding contents around * Remove z-indexes: Clashed with any popovers and wasn’t necessary since dom order signifies this * Added isLoading state (overrides clear button)
ryankeairns
reviewed
Jul 25, 2019
Contributor
ryankeairns
left a comment
There was a problem hiding this comment.
Thanks for cleaning these up!
I see that a number of these still contain either fillRule or fill-rule attributes which we typically remove from new SVGs. That said, I'm not 100% certain they can all be removed without causing potentially altering the shape. It's something we could consider cleaning up in a future PR.
…into feature/icon-svg-def # Conflicts: # CHANGELOG.md
Contributor
|
That last merge brought in some old PRs (filepicker stuff). I'll help @mdefazio clean up later today. |
Contributor
Author
|
Replaced with #2169 |
3 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
Closes #1989
Removes
<use>and<def>from svg icons.Checklist
- [ ] This was checked in mobile-- [ ] This was checked in IE11-- [ ] This was checked in dark mode- [ ] Any props added have proper autodocs- [ ] Documentation examples were added- [ ] This was checked for breaking changes and labeled appropriately- [ ] Jest tests were updated or added to match the most common scenarios- [ ] This was checked against keyboard-only and screenreader scenarios- [ ] This required updates to Framer X components