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(card): update card styles to match design specs #7400

Closed
wants to merge 13 commits into from
Closed

fix(card): update card styles to match design specs #7400

wants to merge 13 commits into from

Conversation

andy-blum
Copy link
Member

Related Ticket(s)

Fixes: #6229 #6255

Description

  • Updates styles of card component to match specs
  • Modernizes spacing techniques of card copy wrapper
  • Reduces width of CTA link host element to prevent focus-on-adjacent-click

Changelog

Changed

  • Updated card styles
  • Updated tag-group styles
  • Fixed bug where clicking next to a card link

@andy-blum andy-blum requested a review from a team as a code owner October 15, 2021 14:26
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 15, 2021 •
edited
Loading

Deploy preview created for package Web Components HTML Test Application:
https://ibmdotcom-web-components-html-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7400/index.html

Built with commit: 82010283e392eefd001842a396c585e830fb71fb

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 15, 2021 •
edited
Loading

Deploy preview created for package React:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/7400/index.html

Built with commit: 82010283e392eefd001842a396c585e830fb71fb

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 15, 2021 •
edited
Loading

Deploy preview created for package Web Components (React wrapper):
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7400/index.html

Built with commit: 82010283e392eefd001842a396c585e830fb71fb

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 15, 2021 •
edited
Loading

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7400/index.html

Built with commit: 82010283e392eefd001842a396c585e830fb71fb

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 15, 2021 •
edited
Loading

Deploy preview created for package NextJS Test Application:
https://ibmdotcom-nextjs-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7400/index.html

Built with commit: 82010283e392eefd001842a396c585e830fb71fb

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 15, 2021 •
edited
Loading

Deploy preview created for package "React (Codesandbox Examples)":
https://react-codesandbox.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7400/index.html

Built with commit: 82010283e392eefd001842a396c585e830fb71fb

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 15, 2021 •
edited
Loading

Deploy preview created for package "Web Components (Codesandbox Examples)":
https://webcomponents-codesandbox.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7400/index.html

Built with commit: 82010283e392eefd001842a396c585e830fb71fb

@ariellalgilmore
Copy link
Member

Hey @andy-blum! Thanks for the updates. There was a recent update though where the tag group is now positioned after the copy: #7315. This shouldn't be changed

@proeung
Copy link
Contributor

proeung commented Oct 15, 2021

@ariellalgilmore is correct. The screenshots that you're referencing in this issue (#6255) are outdated and might have contributed to the changes here and here.

I would keep this PR scope to adjusting the Eyebrow text container and #6229. Also, all of the updated UI components design specs are managed here (https://ibm.ent.box.com/folder/128417762308?s=yqwhif915fwn9avz8gytvkc1p1038ofo). You should have access to this as well.

@proeung proeung added 🛠️ fix needed Needs design approval PRs on feature requests and new components have to get design approval before merge. labels Oct 15, 2021
emyarod
emyarod approved these changes Oct 18, 2021
View reviewed changes
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

looks good to me!

@oliviaflory oliviaflory self-assigned this Oct 19, 2021
@oliviaflory
Copy link
Contributor

@andy-blum thanks! Looking pretty close, here are a few things I noticed or were caught with Percy:

  1. Feature card medium: the arrow position should remain on the right in md–max breakpoints. per the design spec. (Web components)
    Percy

Screen Shot 2021-10-19 at 9 53 04 AM

  1. Tag group: the spacing between tags increased to 16px, it should be 8px according to design specs
    Percy

Screen Shot 2021-10-19 at 10 03 13 AM

Specs / expected
Screen Shot 2021-10-19 at 10 02 39 AM

  1. It looks like the card default got taller, and maybe made the card in card taller? Would you be able to provide any more details on what styles were updated? Just trying to understand if the height increase is intended because we were missing some or the card is now reflecting an aspect ratio, or if it wasn't intentional.

Screen Shot 2021-10-19 at 2 25 59 PM

Screen Shot 2021-10-19 at 2 26 10 PM

@andy-blum
Copy link
Member Author

@oliviaflory The side effects percy found should be addressed now.

jeffchew
jeffchew requested changes Oct 19, 2021
View reviewed changes
Copy link
Member

@jeffchew jeffchew left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey @andy-blum , this branch seems quite out of sync with master. There's a lot of changes that appear will revert a bunch of things that got merged in recently. It's possible this branch is not a fresh branch off master. Can you take a look?

cc: @oliviaflory

@andy-blum
Merge branch 'main' into brow-wax
8201028 
# Conflicts:
#	packages/web-components/.storybook/container.scss
#	packages/web-components/src/components/cta-block/__stories__/cta-block.stories.ts
@andy-blum
Copy link
Member Author

@jeffchew I think I got things untangled. Mind taking a look?

@jeffchew jeffchew mentioned this pull request Oct 20, 2021
Closed
@jeffchew
Copy link
Member

@jeffchew I think I got things untangled. Mind taking a look?

Hmm I'm still seeing a lot of files changed, and most of them are reverting to an older state. Can you take another look?

@andy-blum
Copy link
Member Author

I don't have a good enough understanding of git to fix this, so I'm duplicating my changes onto another branch and will open a new PR.

@andy-blum andy-blum closed this Oct 20, 2021
@andy-blum andy-blum deleted the brow-wax branch October 20, 2021 18:48
@andy-blum andy-blum mentioned this pull request Oct 20, 2021
Merged
@andy-blum
Copy link
Member Author

See #7463

@emyarod emyarod mentioned this pull request Oct 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffchew jeffchew jeffchew requested changes

@emyarod emyarod emyarod approved these changes

@kennylam kennylam Awaiting requested review from kennylam kennylam was automatically assigned from carbon-design-system/carbon-for-ibm-com-members

@oliviaflory oliviaflory Awaiting requested review from oliviaflory oliviaflory was automatically assigned from carbon-design-system/carbon-for-ibm-com-members

@Praveen-sr Praveen-sr Awaiting requested review from Praveen-sr

@DragosRistici DragosRistici Awaiting requested review from DragosRistici

Assignees

@oliviaflory oliviaflory

Labels
🛠️ fix needed Needs design approval PRs on feature requests and new components have to get design approval before merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Card Static] Clicking next the link, CTA link is getting focus
7 participants
@andy-blum @ibmdotcom-bot @ariellalgilmore @proeung @oliviaflory @jeffchew @emyarod