Skip to content

Conversation

@yeshwanth235
Copy link
Contributor

Summary

Found three place which uses hardcoded colors in Storage/RequestForm. Updated them to KDS tokens

References

5094

Reviewer guidance

Check everything is working as expected.
Impacted area in requestForm. Check for below fields

  • What type of organization or group is coordinating the use of Kolibri (if applicable)?
  • What is the licensing of the content you are uploading? (Check all that apply)
  • Organizational affiliation

@MisRob MisRob self-requested a review July 23, 2025 08:03
@MisRob MisRob self-assigned this Jul 23, 2025
@MisRob
Copy link
Member

MisRob commented Jul 24, 2025

Hi @yeshwanth235, thanks! Overall looks aligned with expectations, I will review in detail soon.

@MisRob MisRob changed the title changing hardcode colors to KDS tokens Do not use hardcoded colors in 'Request more space' form Jul 24, 2025
Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yeshwanth235, looks well, thank you! Just one adjustment is needed before we merge.

{{ $tr('fieldRequiredText') }}
</div>
<label :style="{ color: orgSelected ? 'black' : 'gray' }">
<label :style="{ color: orgSelected ? $themePalette.black : $themePalette.grey.v_300 }">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though the output colors are just fine, for black text, it'd be most appropriate to use tokens.text. And for gray one, tokens.textDisabled. palette is more suitable when there's no token available that would hold contextual information about a color usage.

Copy link
Contributor Author

@yeshwanth235 yeshwanth235 Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated it @MisRob

Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All is well, thank you @yeshwanth235

@MisRob MisRob merged commit a8476ad into learningequality:unstable Jul 25, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants