kolibri-design-system icon indicating copy to clipboard operation
kolibri-design-system copied to clipboard

Add $darken_ Utility Functions for Darkening Palette Colors

Open shivam-daksh opened this issue 1 year ago • 3 comments

Description

This PR introduces utility functions $darken1, $darken2, and $darken3 to darken palette colors for use in button hover states and other components. These utilities are added to the KThemePlugin and are now available globally across all Vue components.

Issue addressed

#726

Before/after screenshots

image

Details

New Utility Functions:

  • $darken1(color): Slightly darkens the provided color.
  • $darken2(color): Moderately darkens the provided color.
  • $darken3(color): Heavily darkens the provided color.

These functions are defined in a new file, darkenUtils.js, which handles the color manipulation logic.

Integration:

  • KThemePlugin.js:

    • Imported the darken utility functions from darkenUtils.js.
    • Registered these utilities on the Vue prototype as $darken1, $darken2, and $darken3, making them globally accessible.
  • Example Usage in Vue Components:

    computed: {
      styles() {
        return {
          backgroundColor: this.$darken1(this.$themePalette.red.v_1100),
        };
      }
    }
    
    

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

Testing checklist

  • [ ] Contributor has fully tested the PR manually
  • [ ] If there are any front-end changes, before/after screenshots are included
  • [ ] Critical and brittle code paths are covered by unit tests
  • [ ] The change is described in the changelog section above

Reviewer guidance

  • [ ] Is the code clean and well-commented?
  • [ ] Are there tests for this change?
  • [ ] Are all UI components LTR and RTL compliant (if applicable)?
  • [ ] Add other things to check for here

After review

  • [ ] The changelog item has been pasted to the CHANGELOG.md

Comments

shivam-daksh avatar Aug 17 '24 07:08 shivam-daksh

Hi @MisRob, Please review the PR.

shivam-daksh avatar Aug 17 '24 07:08 shivam-daksh

Thanks @shivam-daksh, the approach looks well to me overall! And very helpful PR description. I will loop in my colleagues for a full review - @ozer550 and/or @AllanOXDi, would you please have a look and also tried to test these utilities on a few colors, here from KDS or even Kolibri? The linked issue contains all the expectations. Thank you :)

MisRob avatar Aug 19 '24 13:08 MisRob

HI @shivam-daksh Thanks for your work here. I am taking look by testing it in few places

AllanOXDi avatar Aug 20 '24 15:08 AllanOXDi

Hey, @AllanOXDi and @MisRob , There was a issue persisting check if changelog.md is changed , So I tried to update the changes in CHANGELOG.md manually. I thought it may disturb the workflows, so I reverted the changes. Waiting for your guidance and approval.

shivam-daksh avatar Aug 20 '24 23:08 shivam-daksh

Hi @shivam-daksh All you need is to add this to the changelog. Something like

 [#728]
  - **Description:** [add the description of your PR  here]
  - **Products impact:**  None 
  - **Addresses:** https://github.com/learningequality/kolibri-design-system/issues/726
  - **Components:**  
  - **Breaking:** N0
  - **Impacts a11y:** No
  - **Guidance:**
  [#728]https://github.com/learningequality/kolibri-design-system/pull/728

AllanOXDi avatar Aug 21 '24 08:08 AllanOXDi

Hey @AllanOXDi , I've changed the changelog file. Please review this again.

shivam-daksh avatar Aug 21 '24 08:08 shivam-daksh

Thanks! look better. But you still need to add the description. Here is an example https://github.com/learningequality/kolibri-design-system/pull/722/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR8. I am sorry I was not clear. If you could also resolve the conflicts- would be good

AllanOXDi avatar Aug 21 '24 09:08 AllanOXDi

@AllanOXDi , Do I've to change other files in documentation as given in this example or I've to change just CHANGELOG.md?

shivam-daksh avatar Aug 21 '24 09:08 shivam-daksh

@AllanOXDi , Do I've to change other files in documentation as given in this example or I've to change just CHANGELOG.md?

No! What you did was good. just supposed to add the description of your work under the description

AllanOXDi avatar Aug 21 '24 10:08 AllanOXDi

Looks good! lets now just resolve the merge conflicts

AllanOXDi avatar Aug 21 '24 10:08 AllanOXDi

Looks good! lets now just resolve the merge conflicts

@AllanOXDi, It's done.

shivam-daksh avatar Aug 21 '24 10:08 shivam-daksh

Looks good! lets now just resolve the merge conflicts

@AllanOXDi, It's done.

Thanks!

AllanOXDi avatar Aug 21 '24 10:08 AllanOXDi

Thanks both @shivam-daksh and @AllanOXDi, and @shivam-daksh thanks for you first contribution. We appreciate it. I will merge soon.

FYI, I made use of one of my better screens and tweaked the values to correspond a bit more closely to resulting hex values mentioned in the issue.

Important part of acceptance criteria is documentation, however we typically do not require volunteers to write documentation, so I jotted it down and am pushing that as well:

Screenshot from 2024-08-21 15-33-03

MisRob avatar Aug 21 '24 13:08 MisRob

Opened related issue https://github.com/learningequality/kolibri-design-system/issues/740

MisRob avatar Aug 21 '24 13:08 MisRob

@shivam-daksh if you're still interested in the follow-up issues

  • https://github.com/learningequality/kolibri/issues/12553
  • https://github.com/learningequality/studio/issues/4634

now it's the right time :)

Just message us in those issues.

MisRob avatar Aug 21 '24 14:08 MisRob

Hi @MisRob, Thanks a lot for the feedback and tweaks! I appreciate you taking the time to document the darken utilities as well. It’s been a great learning experience. Although it was my second contribution (my first contribution was Links leading outside Studio need to have a pop out icon. I would love to working on those issues.

shivam-daksh avatar Aug 21 '24 14:08 shivam-daksh

@shivam-daksh Lovely! You will need to post comment there, otherwise we can't assign. Get some rest too ;)

MisRob avatar Aug 21 '24 15:08 MisRob