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

[Button] 2 bugs, icons in button & icon button focus state #3594

Closed
1 of 2 tasks
shixiedesign opened this issue Jul 30, 2019 · 9 comments · Fixed by #3598
Closed
1 of 2 tasks

[Button] 2 bugs, icons in button & icon button focus state #3594

shixiedesign opened this issue Jul 30, 2019 · 9 comments · Fixed by #3598
Assignees

Comments

@shixiedesign
Copy link
Contributor

shixiedesign commented Jul 30, 2019

  • Vanilla The plus icon in the button is way too thick. I don't think it's our icon.
    image

Should be:
image

  • Vanilla & React Icon button focus state is currently:
    image

Should be with a white border inside to separate the blue focus outline (pls ignore tooltip position difference here):
image

@shixiedesign shixiedesign changed the title [Button] icons in button are wrong [Button] 2 bugs, icons in button & icon button focus state Jul 30, 2019
@tw15egan tw15egan self-assigned this Jul 30, 2019
@tw15egan
Copy link
Collaborator

Seems like the icon issue is only present in the demo environment. Seems to be tied to the @carbon/icons-handlebars package, so we might need to fix it there.

cc @asudoh @joshblack

@asudoh
Copy link
Contributor

asudoh commented Jul 31, 2019

@tw15egan @shixiedesign Could you let me know did you find the problem at? What I'm seeing is that the <svg> content for local vanilla dev env, React Storybook, and our website seem identical. I saw thick plus icon at the-carbon-components.netlify.com. That said, wondering if we have a problem with Netlify deploy.

@tw15egan
Copy link
Collaborator

Hmm, I see the thick plus icon locally... Regardless, it doesn't seem to be an issue in production so I don 't think it's too high of a priority

@tw15egan
Copy link
Collaborator

@asudoh @shixiedesign I've confirmed they are different svgs

local:
<path d="M9 7V3H7v4H3v2h4v4h2V9h4V7z"></path>

website:
<path d="M8.5 7.5V3h-1v4.5H3v1h4.5V13h1V8.5H13v-1z"></path>

@asudoh
Copy link
Contributor

asudoh commented Jul 31, 2019

@tw15egan Could you do me a favor, which is, seeing running yarn install/yarn lerna run build before running the local dev env...? Thanks!

Nvm, it was my local dev env it was stale, which seems to be a sign of a regression. Will take further look.

@asudoh
Copy link
Contributor

asudoh commented Jul 31, 2019

Found that the change came from https://github.com/carbon-design-system/carbon/pull/3520/files#diff-69035b8c650954d2e6454a7e04ea85c5.

@shixiedesign Would you be able to discuss with @chrisconnors-ibm to see what the right content would be? Thanks!

@shixiedesign
Copy link
Contributor Author

I see. 🤕 Ugh 16px icons...I will check with design team. Thanks @asudoh @tw15egan .

@joshblack
Copy link
Contributor

Going to close this one out just due to time, let me know if I should re-open!

@chrisconnors-ibm
Copy link
Contributor

this is two issues.

it's using the correct icon, it's right here: https://github.com/carbon-design-system/carbon/blob/master/packages/icons/svg/16/add.svg

if the react icon button focus state is not as expected, that should be a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants