Skip to content

pfe-cta: Add wind variant styles for "brick" design#566

Merged
kylebuch8 merged 14 commits intomasterfrom
US228968-brick-pfe-cta
Oct 24, 2019
Merged

pfe-cta: Add wind variant styles for "brick" design#566
kylebuch8 merged 14 commits intomasterfrom
US228968-brick-pfe-cta

Conversation

@LyndseyR
Copy link
Copy Markdown
Contributor

@LyndseyR LyndseyR commented Oct 9, 2019

Pfe-cta update | add new brick CTA styles


What has changed and why

Summarize files edited as part of this MR along with a brief description of what was changed/why.

  • Added a pfe-variant="wind" to be called in when using the secondary call-to-action. The style has been requested by the design system.

Testing instructions

Be sure to include detailed instructions on how your update can be tested by another developer.

  1. Pull down branch US228968-brick-pfe-cta
  2. Add a secondary call-to-action <pfe-cta pfe-priority="secondary">
  3. Include the wind variant <pfe-cta pfe-priority="secondary" pfe-variant="wind">

Browser requirements

Your component should work in all of the following environments:

  • Latest 2 versions of Edge
  • Internet Explorer 11 (should be useable, not pixel perfect)
  • Latest 2 versions of Firefox (one on Mac OS, one of Windows OS)
  • Firefox 60.7.2 (or latest version for Red Hat Enterprise Linux distribution)
  • Latest 2 versions of Chrome (one on Mac OS, one of Windows OS)
  • Latest 2 versions of Safari
  • Galaxy S9 Firefox
  • iPhone X Safari
  • iPad Pro Safari
  • Pixel 3 Chrome

Your repository infrastructure updates should work for at least:

  • Node v8.x
  • NPM v7.x

Ready-for-merge Checklist

  • Expected files: all files in this pull request are related to one feature request or issue (no stragglers)?
  • Did browser testing pass?
  • Did you update or add any necessary documentation (README.md, WHY.md, etc.)?
  • Was this feature demo'd and the design review approved?
  • Did you update the CHANGELOG.md file with a summary of this update?

Be sure to share your updates with the patternfly-elements-contribute@redhat.com mailing list!

@starryeyez024 starryeyez024 added the work in progress POC / Not ready for review label Oct 11, 2019
@starryeyez024
Copy link
Copy Markdown
Member

@LyndseyR this PR has been merged, so please pull in the latest from master :)
#564

@LyndseyR LyndseyR added ready: branch testing Test the component from a user-perspective. Try to break it! ready: browser testing Test the component in the supported browser environments. ready: code review Ready for code review! and removed work in progress POC / Not ready for review labels Oct 18, 2019
Comment thread elements/pfe-cta/src/pfe-cta.scss Outdated
Copy link
Copy Markdown
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.

A couple notes about sass functions and mixins to use.

Comment thread elements/pfe-cta/src/pfe-cta.scss Outdated
Comment thread elements/pfe-cta/src/pfe-cta.scss Outdated
Comment thread elements/pfe-cta/src/pfe-cta.scss Outdated
Comment thread elements/pfe-cta/src/pfe-cta.scss Outdated
Comment thread elements/pfe-cta/src/pfe-cta.scss Outdated
Comment thread elements/pfe-cta/src/pfe-cta.scss Outdated
Comment thread elements/pfe-cta/src/pfe-cta--lightdom.scss
Comment thread elements/pfe-cta/src/pfe-cta--lightdom.scss Outdated
@castastrophe castastrophe added needs code updates Code updates have been requested. and removed ready: code review Ready for code review! labels Oct 21, 2019
@castastrophe
Copy link
Copy Markdown
Contributor

One last note - needs the README updated to reflect the new setting.

Comment thread elements/pfe-cta/README.md Outdated
Comment thread elements/pfe-cta/src/pfe-cta.scss
castastrophe
castastrophe previously approved these changes Oct 21, 2019
Copy link
Copy Markdown
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.

I applied code review changes since Lyndsey is out on PTO

…documentation; support button if necessary
…ta storybook to render attributes correctly
@castastrophe castastrophe changed the title US228968 added pfe-cta wind variant styles for brick design pfe-cta: Add wind variant styles for brick design Oct 21, 2019
@castastrophe castastrophe changed the title pfe-cta: Add wind variant styles for brick design pfe-cta: Add wind variant styles for "brick" design Oct 21, 2019
@castastrophe castastrophe removed the needs code updates Code updates have been requested. label Oct 21, 2019
Comment thread elements/pfe-cta/src/pfe-cta.scss
Comment thread elements/pfe-cta/src/pfe-cta.scss
Comment thread elements/pfe-cta/src/pfe-cta.scss
Comment thread elements/pfe-cta/src/pfe-cta.scss
@starryeyez024 starryeyez024 self-assigned this Oct 24, 2019
starryeyez024
starryeyez024 previously approved these changes Oct 24, 2019
Copy link
Copy Markdown
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.

browser testing: https://rally1.rallydev.com/#/270861059696d/detail/testcaseresult/343691073072?fdp=true

Logic Gained Too Much! (besides merge conflicts on changelog / package.json)

@castastrophe castastrophe added styles An issue or PR pertaining only to CSS/Sass and removed ready: branch testing Test the component from a user-perspective. Try to break it! ready: browser testing Test the component in the supported browser environments. labels Oct 24, 2019
Copy link
Copy Markdown
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.

Lower Gaming Third Macedonia

@kylebuch8 kylebuch8 merged commit 1f9ec10 into master Oct 24, 2019
@kylebuch8 kylebuch8 deleted the US228968-brick-pfe-cta branch October 24, 2019 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

styles An issue or PR pertaining only to CSS/Sass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants