Make use of currentColor #62

camillef · 2020-09-23T11:41:05Z

To allow customer branding to cascade

Also includes some miscellaneous fixes picked up from reviewing the update with customer branding in Shipshape

camillef · 2020-09-23T11:45:15Z

assets/stylesheets/bootstrap/mixins/_buttons.scss

@@ -3,10 +3,10 @@
 // Easily pump out default styles, as well as :hover, :focus, :active,
 // and disabled options for all buttons

-@mixin button-variant-plain($color, $background, $border) {


The $border seemed to always be the same as $color so I've removed and used currentColor for all instances of both $color and $border except for the base rule color: $color; which will get overridden by customer branding if needed

assets/stylesheets/bootstrap/_button-groups.scss

danielmatthew · 2020-09-24T08:03:14Z

Looking good: changed the green to rebeccapurple and button borders followed suit…

…but: what about that pesky btn-primary state, and inputs, with a color of #ffffff and #000000 respectively 🤔

danielmatthew · 2020-09-24T08:21:34Z

Do you think there's anything we can do with https://github.com/talis/rl-app/blob/master/src/classes/api/BrandingAPI.class.php to dump out the two core customer colours in the following format?

Your instances of currentColor can be swapped for the appropriate variable.

<style>
:root {
  --customerPrimary: #101;
  --customSecondary: #999;
}
</style>

camillef · 2020-09-24T13:48:10Z

Do you think there's anything we can do with https://github.com/talis/rl-app/blob/master/src/classes/api/BrandingAPI.class.php to dump out the two core customer colours in the following format?
<style>
:root {
  --customerPrimary: #101;
  --customSecondary: #999;
}
</style>

I don't see why not! Might we also need the navbar colour? I'm not sure what the plan is for focus on that...

danielmatthew · 2020-09-24T13:58:59Z

I don't see why not! Might we also need the navbar colour? I'm not sure what the plan is for focus on that...

Ah, blummin' heck. Yes, that would be another property we'd need. Focus/active state-wise, the outline should be left at our be our default white state for now.

camillef · 2020-09-24T16:06:45Z

Ah, blummin' heck. Yes, that would be another property we'd need. Focus/active state-wise, the outline should be left at our be our default white state for now.

Raised a PR: https://github.com/talis/rl-app/pull/2808

camillef · 2020-10-02T16:10:28Z

assets/stylesheets/bootstrap/_buttons.scss

@@ -96,8 +96,8 @@ a.btn {

 // Make a button look and behave like a link
 .btn-link {
-  font-weight: 400;


Redefined two lines below

camillef · 2020-10-02T16:12:14Z

assets/stylesheets/bootstrap/_buttons.scss

  color: $link-color;
+  border-color: transparent;


Ensure btn-link always has a transparent border, even if it is also a btn-danger

Examples are

the cancel button on the action bar when item has been cut

the set importance button when no importance has been set

camillef · 2020-10-02T16:14:25Z

assets/stylesheets/bootstrap/mixins/_alerts.scss

+  .btn-alert {
+    text-decoration: none;
+    &:hover {
+      color: $white !important;
+    }
+  }


Overwrite the two rules immediately above when we have an a styled as a button inside an alert using btn-alert

Should look like

and on hover

camillef · 2020-10-02T16:15:34Z

assets/stylesheets/bootstrap/mixins/_buttons.scss

@@ -199,6 +199,10 @@
    background-image: none;
  }

+  &:hover:active {
+    background-color: $black;


Should favour hover background-color when both hover and active

Was it going transparent again when clicked?

camillef · 2020-10-02T16:20:25Z

Your instances of currentColor can be swapped for the appropriate variable.

Having experimented with CSS variables a little, they really don't play nicely with SCSS... as soon as you want to pass the variable to a function, the sass compiler errors with messages along the lines of argument $color of darken($color, $amount) must be a color

danielmatthew · 2020-10-02T18:35:37Z

Having experimented with CSS variables a little, they really don't play nicely with SCSS...

That's OK. A custom property could be modified with a built-in CSS function instead. We'd modify the l of a hsla() colour to darken it by the required amount.

danielmatthew · 2020-10-05T10:26:44Z

dist/app.css

@@ -226,7 +226,7 @@ a {
    text-decoration: none; }


Don't worry about commiting changes in app.css: there's a task to generate the dist files when the version is bumped.

Oh yeah… the release process. I've been using conventional-commits to help with this. Would you agree that this commit is probably of 'fix', so will bump the patch version?

danielmatthew

Happy with how it's rendering on http://localhost:8000/ ?

To allow customer branding to cascade

Co-authored-by: Dan Matthew <dm@talis.com>

camillef requested a review from danielmatthew September 23, 2020 11:41

camillef commented Sep 23, 2020

View reviewed changes

danielmatthew reviewed Sep 24, 2020

View reviewed changes

assets/stylesheets/bootstrap/_button-groups.scss Outdated Show resolved Hide resolved

camillef commented Oct 2, 2020

View reviewed changes

camillef marked this pull request as ready for review October 2, 2020 16:17

camillef added the 90% label Oct 2, 2020

camillef requested a review from danielmatthew October 2, 2020 16:20

danielmatthew reviewed Oct 5, 2020

View reviewed changes

danielmatthew approved these changes Oct 5, 2020

View reviewed changes

Camille Fenton and others added 10 commits October 5, 2020 11:35

Make use of currentColor

96d4dec

To allow customer branding to cascade

Revert form-control-focus changes

bf23d90

currentColor cannot override color

22a5f99

Fix btn-alert:hover:active background

8a2cede

Fix navbar hover background

ebbc304

Remove commented out rule

972366f

Co-authored-by: Dan Matthew <dm@talis.com>

More use of currentColor

5bfaba6

Navbar outline should use currentColor

a9e7613

Fix btn-alert underline and hover color

79d7148

btn-link should have transparent border

711d8df

camillef force-pushed the use-currentColor branch from da0a006 to 711d8df Compare October 5, 2020 10:55

danielmatthew merged commit 40cb629 into master Oct 5, 2020

danielmatthew deleted the use-currentColor branch October 5, 2020 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make use of currentColor #62

Make use of currentColor #62

camillef commented Sep 23, 2020 •

edited

Loading

camillef Sep 23, 2020

danielmatthew commented Sep 24, 2020

danielmatthew commented Sep 24, 2020 •

edited

Loading

camillef commented Sep 24, 2020

danielmatthew commented Sep 24, 2020

camillef commented Sep 24, 2020

camillef Oct 2, 2020

camillef Oct 2, 2020

camillef Oct 2, 2020

camillef Oct 2, 2020

danielmatthew Oct 5, 2020

camillef Oct 5, 2020

camillef commented Oct 2, 2020 •

edited

Loading

danielmatthew commented Oct 2, 2020

danielmatthew Oct 5, 2020

danielmatthew Oct 5, 2020

camillef Oct 5, 2020

danielmatthew left a comment

Make use of currentColor #62

Make use of currentColor #62

Conversation

camillef commented Sep 23, 2020 • edited Loading

Choose a reason for hiding this comment

danielmatthew commented Sep 24, 2020

danielmatthew commented Sep 24, 2020 • edited Loading

camillef commented Sep 24, 2020

danielmatthew commented Sep 24, 2020

camillef commented Sep 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camillef commented Oct 2, 2020 • edited Loading

danielmatthew commented Oct 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielmatthew left a comment

Choose a reason for hiding this comment

camillef commented Sep 23, 2020 •

edited

Loading

danielmatthew commented Sep 24, 2020 •

edited

Loading

camillef commented Oct 2, 2020 •

edited

Loading