Skip to content

Conversation

@markcaron
Copy link
Contributor

@markcaron markcaron commented Oct 1, 2019

Fixing CTA alignment issue in #478.

  • Updated demo to show multiple CTAs in a single card (for testing)
  • Tested in FF, Chrome and Edge

@starryeyez024
Copy link
Member

starryeyez024 commented Oct 11, 2019

@alazzara can you do branch testing on this?
@starryeyez024 Sure

@starryeyez024 starryeyez024 changed the title Pfe card cta alignment pfe-card cta alignment Oct 11, 2019
castastrophe
castastrophe previously approved these changes Oct 11, 2019
Copy link
Contributor

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

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

Looks great! I added a minor lightdom fallback style for IE11 to limit image sizes to 100% since the widths set by variables don't apply there.

# resolved conflicts:
#	elements/pfe-card/src/pfe-card.scss
#	package-lock.json
@starryeyez024
Copy link
Member

@markcaron

  1. I merged in master but it looks like this branch contains a lot of overlap with what was in this PR:: Fix image distortion in cards #417 - so I'm not sure what should stay or go
  2. This PR also backs out support for the attribute color (in favor of pfe-color only) and storybook only references color, which needs to be fixed
  3. I can't figure out why the package-lock.json is diffing; I copied what was in master into this branch.

@markcaron
Copy link
Contributor Author

@kylebuch8, beyond @starryeyez024's question about the overflow attributes yesterday, I believe everything else has been addressed.

@starryeyez024
Copy link
Member

@markcaron this branch still has overflow attribute changes, and it introduces regressions to the image alignment:
image

Master:
image

@starryeyez024
Copy link
Member

Storybook knob for the image overflow settings does not have any effect on the pfe-card component in the demo pane.
http://localhost:9001/?path=/story/card--pfe-card

starryeyez024
starryeyez024 previously approved these changes Oct 25, 2019
# Conflicts:
#	CHANGELOG-prerelease.md
#	elements/pfe-card/package.json
#	package-lock.json
Copy link
Member

@starryeyez024 starryeyez024 left a comment

Choose a reason for hiding this comment

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

Louisiana Granny Thumbnail Murray

@kylebuch8 kylebuch8 merged commit 79614bc into master Oct 25, 2019
@kylebuch8 kylebuch8 deleted the pfe-card-cta-alignment branch October 25, 2019 19:06
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.

6 participants