-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
style: updates button styles to match Pine #2058
style: updates button styles to match Pine #2058
Conversation
Dependency ReviewThe following issues were found:
License Issuespackages/sage-react/package.json
yarn.lock
Denied Licenses: LGPL-2.0, AAL, Adobe-2006, Afmparse, Artistic-1.0, Artistic-1.0-cl8, Artistic-1.0-Perl, Beerware, blessing, Borceux, CECILL-B, ClArtistic, Condor-1.1, Crossword, CrystalStacker, diffmark, DOC, EFL-1.0, EFL-2.0, Fair, FSFUL, FSFULLR, Giftware, HPND, IJG, Leptonica, LPL-1.0, LPL-1.02, MirOS, mpich2, NASA-1.3, NBPL-1.0, Newsletr, NLPL, NRL, OGTSL, OLDAP-1.1, OLDAP-1.2, OLDAP-1.3, OLDAP-1.4, psutils, Qhull, Rdisc, RSA-MD, Spencer-86, Spencer-94, TU-Berlin-1.0, TU-Berlin-2.0, Vim, W3C-19980720, W3C-20150513, Wsuipa, WTFPL, xinetd, Zed, Zend-2.0, ZPL-1.1, AGPL-1.0-only, AGPL-1.0-or-later, AGPL-3.0-only, AGPL-3.0-or-later, APSL-1.0, APSL-1.1, APSL-1.2, APSL-2.0, CPAL-1.0, EUPL-1.0, EUPL-1.1, EUPL-1.2, NPOSL-3.0, OSL-1.0, OSL-1.1, OSL-2.0, OSL-2.1, OSL-3.0, RPSL-1.0, SSPL-1.0, CAL-1.0, CAL-1.0-Combined-Work-Exception, Parity-6.0.0, Parity-7.0.0, RPL-1.1, RPL-1.5, EPL-1.0, EPL-2.0, ErlPL-1.1, IPL-1.0, LGPL-2.0-only, LGPL-2.0-or-later, LGPL-2.1-only, LGPL-2.1-or-later, LGPL-2.1, LGPL-3.0-only, LGPL-3.0-or-later, LGPL-3.0, MPL-1.0, MPL-1.1, MPL-2.0, MPL-2.0-no-copyleft-exception, MS-RL, SPL-1.0, BSD-Protection, copyleft-next-0.3.0, copyleft-next-0.3.1, GPL-1.0-only, GPL-1.0-or-later, GPL-1.0, GPL-2.0-only, GPL-2.0-or-later, GPL-2.0, GPL-3.0-only, GPL-3.0-or-later, GPL-3.0, QPL-1.0, Sleepycat Scanned Manifest Filespackages/sage-react/package.json
yarn.lock
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 comment on mixins, otherwise LGTM.
@@ -53,15 +53,15 @@ | |||
/// Sets up styles for sage "copy" (system) text. | |||
/// | |||
@mixin sage-copy-text() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was my understanding that we weren't adjusting mixins but instead replacing them within the component when possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was the intention, and I still stand by that for the most part, but seeing how fragmented a lot of these styles are makes me think in some cases it won't make much of a difference. Here, this mixin isn't used anywhere else in the repo or the main app, so it comes down to either changing the styles in the mixin itself or copy and paste the styles within two different selectors, neither of which I'm a huge fan of.
Description
Updates button styles to match Pine.
Screenshots
Testing in
sage-lib
Testing in
kajabi-products
Related
DSS-1256