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

Reorganize files for CTA so it's consistent with other components #345

Merged
merged 6 commits into from
Mar 19, 2019
Merged

Reorganize files for CTA so it's consistent with other components #345

merged 6 commits into from
Mar 19, 2019

Conversation

yvonnetangsu
Copy link
Member

@yvonnetangsu yvonnetangsu commented Mar 15, 2019 •
edited
Loading

READY FOR REVIEW

Summary

  • Other components have variant styles in their own separate scss files. This PR makes CTA consistent with other components by using the same file structure.
  • Some small cleanup. This component was my first PR - it could use some improvement.
  • Create a centering mixin (recommended by many sites as a good utility mixin) and use it in this component (the .cta--button-center variant).

Needed By (Date)

  • Hopefully before v.5 release so everything's consistent

Urgency

  • Not urgent

Steps to Test

  1. Pull this branch and run grunt styleguide
  2. Check that all the CTA variants are still the same.
  3. Check that the @centering mixin is working.

Affected Projects or Products

  • Decanter

@yvonnetangsu yvonnetangsu requested a review from sherakama March 15, 2019 23:56
@sherakama
Copy link
Member

This looks good. Thanks for this work. Perhaps move the mixin to the layouts directory instead of creating a new folder. May as well start to fill out those directories.

@yvonnetangsu
Copy link
Member Author

This looks good. Thanks for this work. Perhaps move the mixin to the layouts directory instead of creating a new folder. May as well start to fill out those directories.

@sherakama, @centering has been moved under Layout. Let me know if you approve 😄

@yvonnetangsu yvonnetangsu added the maintenance Maintenance Tasks label Mar 18, 2019
@sherakama sherakama merged commit b25bdce into master Mar 19, 2019
@sherakama sherakama deleted the reorganize-cta-scss branch March 19, 2019 02:01
JBCSU added a commit that referenced this pull request Mar 19, 2019
@JBCSU
Merge branch 'master' into 90-basic-webpack
13be2bb 
* master:
  Reorganize files for CTA so it's consistent with other components (#345)
  Update main-nav.twig (#331)
  get rid of ID (#342)
  5 Lines Lockup + Variants (#289)
  Update _alert--info.scss (#330)

# Conflicts: all resolved using "mine"
#	core/dist/css/decanter.css
#	package-lock.json
#	package.json
yvonnetangsu added a commit that referenced this pull request Mar 20, 2019
@yvonnetangsu
Merge branch 'master' into 329-card-tweak
fcfee66 
* master:
  Use %placeholder and @extend on button mixins to see if it makes code less repetitive (#347)
  Cut down on media queries generated by @modular-spacing (#343)
  Main Nav Light on Dark Background Color Variant (#344)
  Reorganize files for CTA so it's consistent with other components (#345)
  Update main-nav.twig (#331)
  get rid of ID (#342)
  5 Lines Lockup + Variants (#289)
  Update _alert--info.scss (#330)

# Conflicts:
#	core/css/decanter.css
#	core/scss/utilities/mixins/button/_button-big.scss
#	core/scss/utilities/mixins/button/_button-primary.scss
#	core/scss/utilities/mixins/button/_button-secondary.scss
#	core/scss/utilities/mixins/index.scss
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sherakama sherakama sherakama approved these changes

Assignees
No one assigned
Labels
maintenance Maintenance Tasks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yvonnetangsu @sherakama