Introduce new attention level designs for button and related theming for card #2735

RasmusKjeldgaard · 2023-01-10T08:19:26Z

Which issue does this PR close?

This PR closes #2684 closes #2602

What is the new behavior?

As card implements the themeColor directive, card is now also responsible for setting the correct theme CSS custom properties that buttons (form-field and segmented-control too, down the line) implement as background-color and color.

Otherwise the button would not have the correct theme when themeColor was not set on card, as the card would have no .kirby-color-brightness-white class applied in that situation. Thus the button would not be able to adapt correctly in its host-context() css selectors.

See both issues linked above for additional context, especially #2602 for the design context.

All in all, this PR includes:

The slight rework of theming as described above
Updates to attention level 2 & 3 for button and dropdown (dropdown is implicitly changed because it uses button directly, and should no longer differ from the button attention level designs)
Theme mixins that sets background and color variables to be shared between form-field, segmented control etc.
Interaction state values have been changed according to design in sketch: https://www.sketch.com/s/af90babd-c91f-4870-b097-ce26b590ed59/p/5F7B74E2-9036-46D5-95B0-6777F9BF44BF/canvas

Does this PR introduce a breaking change?

Yes
No

Are there any additional context?

Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Reminders

Make sure you have implemented tests following the guidelines in: "The good: Test".
Make sure you have updated the cookbook with examples and showcases (for bug fixes, enhancements & new components).

Review

Determine if your changes are a fix, feature or breaking-change, and add the matching label to your PR. If it is tooling, dependency updates or similar, add ignore-for-release.
Do a self-review.
Request that the changes are code-reviewed
Request that the changes are UX reviewed (only necessary if your PR introduces visual changes)

When the pull request has been approved it will be merged to develop by Team Kirby.

RasmusKjeldgaard · 2023-01-16T09:30:12Z

apps/cookbook/src/app/examples/dropdown-example/examples/attention-level.ts

@@ -2,20 +2,69 @@ import { Component, Input } from '@angular/core';



All the changes in this file are just to have a good showcase for the three different versions of dropdown on each theme. It is not the most beautiful solution, but at least it mirrors 100% how we do it for button.

RasmusKjeldgaard · 2023-01-16T09:37:05Z

libs/core/src/scss/themes/_component-themes.scss

+@use '../utils';
+
+@mixin apply-dark-theme {
+  --kirby-interactive-background-color: #{utils.get-color('white-overlay')};


Can we come up with a better prefix than --kirby-interactive- here?
I want it to signal that this is used for our "interactive" components, like buttons, segmented control, form field etc. So it is mostly form-controls, but not quite isolated to those, as segmented control is not a form-control I guess 🤷

That's a really good question 🤔 The only suggestion I can come up with is --kirby-interactive-element-background-color or --kirby-interactive-component-background-color. If you hadn't explained the reasoning for the naming, then I think it would have been unclear to me, what it targets 🙂 I know that the naming becomes a bit long, but I'm a firm believer in "clarity over brevity", when it comes to naming variables and functions 😄

Agree! I changed it to --kirby-interactive-element-background-color and --kirby-interactive-element-background-color

RasmusKjeldgaard · 2023-01-16T09:38:39Z

libs/designsystem/src/lib/components/avatar/avatar.component.spec.ts

@@ -184,8 +184,12 @@ describe('AvatarComponent', () => {



This change seems unrelated, but the approach chosen here for looping over each color broke when I introduced the overlay variants in libs/core/src/scss/themes/_colors.scss.

Instead it is now aligned with the actual implementation inside the component.

mark-drastrup

Really great work! 😄 And nice updates to the documentation as well! I only have some minor comments.

apps/cookbook/src/app/examples/dropdown-example/examples/attention-level.ts

mark-drastrup · 2023-01-17T08:00:29Z

libs/core/src/scss/themes/_component-themes.scss

+@use '../utils';
+
+@mixin apply-dark-theme {
+  --kirby-interactive-background-color: #{utils.get-color('white-overlay')};


That's a really good question 🤔 The only suggestion I can come up with is --kirby-interactive-element-background-color or --kirby-interactive-component-background-color. If you hadn't explained the reasoning for the naming, then I think it would have been unclear to me, what it targets 🙂 I know that the naming becomes a bit long, but I'm a firm believer in "clarity over brevity", when it comes to naming variables and functions 😄

Co-authored-by: mark-drastrup <mark.drastrup@gmail.com>

mark-drastrup

Beautiful! You're a wizard!

…for card (#2735) Co-authored-by: mark-drastrup <mark.drastrup@gmail.com>

RasmusKjeldgaard mentioned this pull request Jan 10, 2023

Change Attention Level 3 button style #2603

Closed

7 tasks

RasmusKjeldgaard force-pushed the enhancement/2684-set-default-themecolor branch from 3300f2b to 505a42d Compare January 12, 2023 08:05

github-actions bot temporarily deployed to pr-2684-set-default-themecolor January 13, 2023 12:17 Inactive

github-actions bot temporarily deployed to pr-2684-set-default-themecolor January 16, 2023 09:02 Inactive

RasmusKjeldgaard marked this pull request as ready for review January 16, 2023 09:23

RasmusKjeldgaard requested a review from jkaltoft as a code owner January 16, 2023 09:23

RasmusKjeldgaard force-pushed the enhancement/2684-set-default-themecolor branch from c5b2c6b to 558a347 Compare January 16, 2023 09:23

github-actions bot temporarily deployed to pr-2684-set-default-themecolor January 16, 2023 09:28 Inactive

RasmusKjeldgaard commented Jan 16, 2023

View reviewed changes

RasmusKjeldgaard changed the title ~~Introduce new att-level 3 button and related theming for card~~ Introduce new attention level designs for button and related theming for card Jan 16, 2023

RasmusKjeldgaard added the feature Add this PR to the changelog as a feature label Jan 16, 2023

github-actions bot temporarily deployed to pr-2684-set-default-themecolor January 16, 2023 13:38 Inactive

mark-drastrup suggested changes Jan 17, 2023

View reviewed changes

RasmusKjeldgaard added 15 commits January 17, 2023 14:51

Make att. lvl. 3 accept custom properties from ancestors

6b0add2

First theming on card in place

af0a616

Use correct design tokens for light context

eba8b6b

Introduce component theme mixins

8c2ca0f

WIP

7b4a8b7

Simplify solution and use existing btn css props

a0a93c9

Introduce themecolor tests

398f726

Group related tests

7ced526

Update interaction states on button

8015601

Update tests to match design spec

feafe86

Remove border styles as it is no longer needed

45579a1

Add theme color variants to color maps

5d9a813

Update failing tests to match design

3900f7c

Revert temporary cookbook changes

2f7a6f9

Remove finished todos

ed8e0fa

RasmusKjeldgaard added 10 commits January 17, 2023 14:51

Adjust interaction states

39fd259

Remove custom dropdown styles to align with button

2b33c07

Adjust action sheet to use btn out of the box

0bd1b0f

Add elevation to light theme

350d626

Remove shadow on no-decoration btn on light bg

6f1b193

Reword it block to match actual test

c17ce77

Align action-sheet test with new design

c5df0de

Adjust color opacity to match spec

e2a0251

Update interaction states for hover and active

95de1e6

Adjust attention level 3 states on dark bg

751b767

RasmusKjeldgaard force-pushed the enhancement/2684-set-default-themecolor branch from d10e080 to 751b767 Compare January 17, 2023 13:51

Update cookbook dropdown placeholder text

f7138bf

Co-authored-by: mark-drastrup <mark.drastrup@gmail.com>

github-actions bot temporarily deployed to pr-2684-set-default-themecolor January 17, 2023 13:56 Inactive

github-actions bot temporarily deployed to pr-2684-set-default-themecolor January 17, 2023 13:57 Inactive

Update css property names to be more descriptive

fb6c392

mark-drastrup approved these changes Jan 17, 2023

View reviewed changes

RasmusKjeldgaard merged commit d5a30c3 into develop Jan 17, 2023

RasmusKjeldgaard deleted the enhancement/2684-set-default-themecolor branch January 17, 2023 14:58

RasmusKjeldgaard mentioned this pull request Feb 6, 2023

[BUG] Only show button drop shadow in att. lvl 3 on light background #2836

Closed

9 tasks

SigurdVilstrup pushed a commit that referenced this pull request Mar 28, 2023

Introduce new attention level designs for button and related theming …

f7d2d68

…for card (#2735) Co-authored-by: mark-drastrup <mark.drastrup@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce new attention level designs for button and related theming for card #2735

Introduce new attention level designs for button and related theming for card #2735

RasmusKjeldgaard commented Jan 10, 2023 •

edited

Loading

RasmusKjeldgaard Jan 16, 2023

RasmusKjeldgaard Jan 16, 2023 •

edited

Loading

mark-drastrup Jan 17, 2023

RasmusKjeldgaard Jan 17, 2023

RasmusKjeldgaard Jan 16, 2023

mark-drastrup left a comment

mark-drastrup Jan 17, 2023

mark-drastrup left a comment

		@@ -2,20 +2,69 @@ import { Component, Input } from '@angular/core';

Introduce new attention level designs for button and related theming for card #2735

Introduce new attention level designs for button and related theming for card #2735

Conversation

RasmusKjeldgaard commented Jan 10, 2023 • edited Loading

Which issue does this PR close?

What is the new behavior?

Does this PR introduce a breaking change?

Are there any additional context?

Checklist:

Reminders

Review

RasmusKjeldgaard Jan 16, 2023

Choose a reason for hiding this comment

RasmusKjeldgaard Jan 16, 2023 • edited Loading

Choose a reason for hiding this comment

mark-drastrup Jan 17, 2023

Choose a reason for hiding this comment

RasmusKjeldgaard Jan 17, 2023

Choose a reason for hiding this comment

RasmusKjeldgaard Jan 16, 2023

Choose a reason for hiding this comment

mark-drastrup left a comment

Choose a reason for hiding this comment

mark-drastrup Jan 17, 2023

Choose a reason for hiding this comment

mark-drastrup left a comment

Choose a reason for hiding this comment

RasmusKjeldgaard commented Jan 10, 2023 •

edited

Loading

RasmusKjeldgaard Jan 16, 2023 •

edited

Loading