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

Added minimize icon #2457

Merged
merged 5 commits into from
Oct 22, 2019
Merged

Added minimize icon #2457

merged 5 commits into from
Oct 22, 2019

Conversation

andreadelrio
Copy link
Contributor

@andreadelrio andreadelrio commented Oct 21, 2019

Summary

Added minimize icon. Fixes #837

image3

image (1)

Checklist

- [ ] Checked in dark mode
- [ ] Checked in mobile
- [ ] Checked in IE11 and Firefox
- [ ] Props have proper autodocs

  • Added documentation examples
    - [ ] Added or updated jest tests
    - [ ] Checked for breaking changes and labeled appropriately
    - [ ] Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

Looking good! One small change and then rerun the compile and snapshots. Thanks :)

CHANGELOG.md Outdated
@@ -7,6 +7,7 @@
- Added `external` prop to `EuiLink` ([#2442](https://github.com/elastic/eui/pull/2442))
- Added disabled state to `EuiBadge` ([#2440](https://github.com/elastic/eui/pull/2440))
- Changed `EuiLink` to appear non interactive when passed the `disabled` prop and an `onClick` handler ([#2423](https://github.com/elastic/eui/pull/2423))
- Added `minimize` icon to glyph set ([#2457](https://github.com/elastic/eui/pull/2457))
Copy link
Contributor

Choose a reason for hiding this comment

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

Re-word this to be 'Added minimize glyph to EuiIcon

@@ -0,0 +1,3 @@
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
<path fill-rule="evenodd" d="M1.14644661,14.1464466 L5.14644661,10.1464466 C5.34170876,9.95118446 5.65829124,9.95118446 5.85355339,10.1464466 C6.02711974,10.320013 6.04640489,10.5894374 5.91140884,10.7843055 L5.85355339,10.8535534 L1.85355339,14.8535534 C1.65829124,15.0488155 1.34170876,15.0488155 1.14644661,14.8535534 C0.972880258,14.679987 0.953595107,14.4105626 1.08859116,14.2156945 L1.14644661,14.1464466 L5.14644661,10.1464466 L1.14644661,14.1464466 Z M6.5,8 C7.32842712,8 8,8.67157288 8,9.5 L8,12.5 C8,12.7761424 7.77614237,13 7.5,13 C7.22385763,13 7,12.7761424 7,12.5 L7,9.5 C7,9.22385763 6.77614237,9 6.5,9 L3.5,9 C3.22385763,9 3,8.77614237 3,8.5 C3,8.22385763 3.22385763,8 3.5,8 L6.5,8 Z M8.5,3 C8.77614237,3 9,3.22385763 9,3.5 L9,6.5 C9,6.77614237 9.22385763,7 9.5,7 L12.5,7 C12.7761424,7 13,7.22385763 13,7.5 C13,7.77614237 12.7761424,8 12.5,8 L9.5,8 C8.67157288,8 8,7.32842712 8,6.5 L8,3.5 C8,3.22385763 8.22385763,3 8.5,3 Z M10.1514466,5.14644661 L14.1514466,1.14644661 C14.3467088,0.951184464 14.6632912,0.951184464 14.8585534,1.14644661 C15.0321197,1.32001296 15.0514049,1.58943736 14.9164088,1.7843055 L14.8585534,1.85355339 L10.8585534,5.85355339 C10.6632912,6.04881554 10.3467088,6.04881554 10.1514466,5.85355339 C9.97788026,5.67998704 9.95859511,5.41056264 10.0935912,5.2156945 L10.1514466,5.14644661 L14.1514466,1.14644661 L10.1514466,5.14644661 Z"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can safely remove the fill-rule="evenodd" here and then run the icon compile again which should remove it from the minimize.js file too. Same for updating the snapshots.

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

LGTM. There are other arrow-based icons in the set with different styles (connected point), but looking at this one alongside the expand icon reveals the strong similarities.

For fun, I threw it into an icon button, presuming it may get used in that manner, and it also looks good there too:

Screenshot 2019-10-22 09 06 39

@andreadelrio andreadelrio merged commit 9eee6a4 into elastic:master Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EuiIcon for minimize needed
2 participants