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 muted bell icon #2714

Merged
merged 8 commits into from
Jan 8, 2020
Merged

Added muted bell icon #2714

merged 8 commits into from
Jan 8, 2020

Conversation

andreadelrio
Copy link
Contributor

@andreadelrio andreadelrio commented Dec 24, 2019

Summary

Added bellSlash icon. Fixes #2485

image

image

Checklist

- [ ] Check against all themes for compatability in both light and dark modes
- [ ] 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

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

The position of the slash create a bit of an awkward area on the bottom left where the curve of the bell is cut in half creating less than 1px strokes. Consider moving the slash to start in that area and would thus completely remove it. Also, to further indicate no sound, maybe remove the knocker portion? Not sure how that will look when toggling.

Screen Shot 2020-01-02 at 11 56 47 AM

@@ -33,6 +33,7 @@ export const iconTypes = [
'asterisk',
'beaker',
'bell',
'bellMuted',
Copy link
Contributor

Choose a reason for hiding this comment

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

Being picky here about naming, but I think "Muted" may not be generic enough. Perhaps bellSlash instead similar to the way we name things like "empty" which is more indicating the visual of icon versus the perceived meaning.

@andreadelrio
Copy link
Contributor Author

image
image

@cchaos Came up with two iterations. Does the slash look too long on the left?
I think I like it better with the knocker. I was looking at references at the Noun Project and they all keep the knocker for muted bells. @mdefazio what do you think?

@cchaos
Copy link
Contributor

cchaos commented Jan 2, 2020

Yeah I agree that the one without the knocker is weird. I was just trying to think of something that might further make it obvious that it's "off". Your slash looks longer on the left because it is. You'll need to move the top right corner up and right one pixel to align correctly.

Screen Shot 2020-01-02 at 16 00 50 PM

@mdefazio
Copy link
Contributor

mdefazio commented Jan 2, 2020

I agree that it needs the knocker as well. Icons are fun aren't they :D

@andreadelrio
Copy link
Contributor Author

image
image

I think it looks cleaner now. I also updated the name to be more generic.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

LGTM! I also tested this as a toggle button and it swaps pretty seamlessly.

Screen Recording 2020-01-03 at 08 57 AM

@@ -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="M9.49999997,14 L9.49451423,14.1492623 C9.41816509,15.1841222 8.55436174,16 7.49999997,16 C6.49835626,16 5.66869157,15.2636703 5.52276937,14.3027743 L5.5054857,14.1492623 L5.49999997,14 L9.49999997,14 Z M14.8796283,1.17460431 C15.0393715,1.36097135 15.0379359,1.63108127 14.8883918,1.81502169 L14.8253957,1.8796283 L0.825395687,13.8796283 C0.615732765,14.0593394 0.300082774,14.0350586 0.120371699,13.8253957 C-0.0393714794,13.6390286 -0.0379359166,13.3689187 0.111608206,13.1849783 L0.174604313,13.1203717 L14.1746043,1.1203717 C14.3842672,0.940660623 14.6999172,0.964941392 14.8796283,1.17460431 Z M13.0359909,5.511 L13.0908419,5.8640426 L13.1387368,6.23155682 C13.2053316,6.78579287 13.2415864,7.21274308 13.2841185,7.9558329 L13.3644259,9.44739715 C13.5101053,11.8179561 13.7663789,12.5 14.5,12.5 C14.7761424,12.5 15,12.7238576 15,13 C15,13.2761424 14.7761424,13.5 14.5,13.5 C13.8966306,13.5 13.468614,13.3536377 13.1599064,13.001441 L4.18499094,13.001 L5.36699094,12.001 L12.6840504,12.0015554 C12.5121773,11.3565474 12.4199339,10.4726104 12.35354,9.29267387 L12.2640974,7.6552099 C12.2310573,7.14140828 12.1981058,6.78551842 12.1458783,6.35085462 L12.1349909,6.274 L13.0359909,5.511 Z M7.5,-5.86197757e-14 C8.50862459,-5.86197757e-14 9.34286466,0.74662914 9.48018176,1.71734903 C9.95574215,1.8302978 10.3866115,1.98772981 10.7717509,2.18930818 L9.93350525,2.89926049 C9.47763954,2.71665692 8.94814969,2.59704985 8.34251437,2.53966933 C8.44186217,2.38445627 8.49999996,2.19901754 8.49999996,2 C8.49999996,1.44771525 8.0522847,1 7.49999996,1 C6.94771521,1 6.49999996,1.44771525 6.49999996,2 C6.49999996,2.19901754 6.55813775,2.38445627 6.65835635,2.54025923 C4.45590623,2.74829123 3.2605094,3.779471 2.95048592,5.67021349 L2.8996398,6.00429325 L2.85448266,6.35086072 C2.77324253,7.02691339 2.73863742,7.51227557 2.6777396,8.69805232 L2.66426105,8.96319272 L2.65899094,9.053 L1.60119881,9.95009191 C1.61933378,9.71568353 1.63578942,9.46162924 1.65105858,9.18657684 L1.72769592,7.76040888 C1.76561948,7.13387121 1.80108764,6.73532707 1.86162562,6.23155072 C1.89158283,5.98225704 1.92538416,5.74186107 1.96366374,5.50840508 C2.29837467,3.46710097 3.50986254,2.19497679 5.51936282,1.71608575 L5.52276937,1.69722565 C5.66869157,0.736329706 6.49835626,-5.86197757e-14 7.5,-5.86197757e-14 Z"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the fill-rule

@andreadelrio andreadelrio merged commit 51d90c8 into elastic:master Jan 8, 2020
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.

Icon of Bell with slash through it for a 'disabled' or 'mute' state
3 participants