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.

Sign up for GitHub

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

Jump to bottom

fix: update icon sizing for resource cards #273

Merged
merged 15 commits into from
Jul 30, 2019
Merged

fix: update icon sizing for resource cards #273

merged 15 commits into from
Jul 30, 2019

Conversation

alisonjoseph
Copy link
Member

@alisonjoseph alisonjoseph commented Jul 23, 2019 •
edited
Loading

ref carbon-design-system/design-language-website#99

Updates the icon size to be a max 64px width and height for the resource cards. We were previously assuming 32x32 icons, but that is not the case everywhere.

@vercel
Copy link

vercel bot commented Jul 23, 2019 •
edited
Loading

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://gatsby-theme-carbon-git-fork-alisonjoseph-icon-size-bug.carbon-design-system.now.sh

vpicone
vpicone suggested changes Jul 24, 2019
View reviewed changes
Copy link
Contributor

@vpicone vpicone left a comment

Choose a reason for hiding this comment

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

Looks like we lost the Github SVG in the second card. Probably need a min-width or somethin.

@alisonjoseph
Copy link
Member Author

Oh good point. I added a min-wdith back in, I'll just have to do a specific override for the IDL site, they have an icon that's 21x52px wide on this page https://design-language-website.netlify.com/elements/logos/rebus so its displaying too large. 😓

@vpicone
Copy link
Contributor

vpicone commented Jul 25, 2019

Hmmmm still not working for me

@alisonjoseph
Copy link
Member Author

me either 😓 -- I'll look at it tomorrow sometime

@vpicone
Copy link
Contributor

vpicone commented Jul 29, 2019

Are we good to go here? Did you wanna test it on IDL first?

@alisonjoseph
Copy link
Member Author

Just tested on IDL and seems I need to remove the min-width. See any issues w/ that? I didn't on IDL, but might be missing something.

@vpicone
Copy link
Contributor

vpicone commented Jul 29, 2019

Removing min-width didn't work for me. Adding position: static to the gatsby responsive image wrapper did though.

@vercel vercel bot temporarily deployed to staging July 29, 2019 18:32 Inactive
@alisonjoseph
Copy link
Member Author

@vpicone ok I think its really working this time

@vercel vercel bot temporarily deployed to staging July 29, 2019 18:33 Inactive
@alisonjoseph
chore: cleanup style order
bd41b35
@alisonjoseph
Merge branch 'icon-size-bug' of github.com:alisonjoseph/gatsby-theme-…
f73fdb6 
…carbon into icon-size-bug

* 'icon-size-bug' of github.com:alisonjoseph/gatsby-theme-carbon:
  v1.6.2
@vpicone vpicone merged commit c8db707 into carbon-design-system:master Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vpicone vpicone vpicone approved these changes

Assignees
No one assigned
Labels
status: ready for review 👀
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alisonjoseph @vpicone