Skip to content

Conversation

@starryeyez024
Copy link
Member

@starryeyez024 starryeyez024 commented Apr 29, 2020

Without theme:
image

With RH theme:
image

Design spec

Testing instructions

  1. View the CTA demo page & compare the default CTA to the mockups
    • observe the spacing around the arrow, it should be closer to the text, as shown in the mockup
    • Test the hover and focus states

Ready-for-merge Checklist

Check off items as they are completed. Feel free to delete items if they are not applicable to your PR.

  • Expected files: all files in this pull request are related to one request or issue (no stragglers or scope-creep).
  • Browser testing passed.
  • Approved by designer.

@castastrophe
Copy link
Contributor

@starryeyez024 Can you add a changelog when you get a chance?

@starryeyez024 starryeyez024 requested a review from castastrophe May 1, 2020 22:47
@patternfly patternfly deleted a comment from castastrophe May 1, 2020
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 good, just need changelog fixed

castastrophe
castastrophe previously approved these changes May 4, 2020
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.

Approved, ready to merge after browser testing and designer sign-off. Probs don't need a demo for this though :D

Seems to be based on fractional units?
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.

Code looks great! 👍

@starryeyez024 starryeyez024 merged commit 6eed4c3 into master May 6, 2020
@bennypowers bennypowers deleted the fix-cta-spacing branch June 25, 2024 07:35
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.

3 participants