These warnings are not highlighted because Claro does not attach "system.admin.css".

CommentFileSizeAuthor
#10 patch.png100.12 KBemarinho
#8 interdiff_3-8.txt926 bytesnitin shrivastava
#8 3311776-8.patch980 bytesnitin shrivastava
#5 Claro_incompatible.png92.46 KBemarinho
#3 claro-admin-missing-3311776-3.patch966 byteschi
#2 claro-admin-missing-3311776-2.patch974 byteschi
#2 claro_admin_modules.png157.7 KBchi

Comments

Chi created an issue. See original summary.

chi’s picture

chi commented
Status: Active » Needs review
StatusFileSize
new claro_admin_modules.png157.7 KB
new claro-admin-missing-3311776-2.patch974 bytes
chi’s picture

chi commented
StatusFileSize
new claro-admin-missing-3311776-3.patch966 bytes
emarinho’s picture

emarinho commented
Assigned: Unassigned » emarinho

I'll review it.

emarinho’s picture

emarinho commented
Assigned: emarinho » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Claro_incompatible.png92.46 KB

Everything's fine!! Nice work!

chi’s picture

chi commented
Issue summary: View changes
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/themes/claro/css/components/system-admin--modules.pcss.css
@@ -180,3 +180,7 @@ td.module-list__description {
+  color: var(--color-maximumred);

Thanks for working on this. Just one small change: this should actually use color: var(--color-maximumred); instead of color: var(--color-red);.

We'll be changing the color naming structure in #3257287: [Policy] Create CSS standards for core use of CSS custom properties (aka CSS Variables) to make it simpler and we'll document it, but for now all main colors need to follow the current pattern, which is using the Primary (line 5 in variables.pcss.css) or Secondary(line10 in variables.pcss.css) palettes.

nitin shrivastava’s picture

nitin shrivastava commented
StatusFileSize
new 3311776-8.patch980 bytes
new interdiff_3-8.txt926 bytes

@ckrina made changes as per comment #7. Needs review.

emarinho’s picture

emarinho commented
Assigned: Unassigned » emarinho
Status: Needs work » Needs review

Thanks, @ckrina and @Nitin shrivastava! I'll review now the new changes.

emarinho’s picture

emarinho commented
Assigned: emarinho » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new patch.png100.12 KB

Nice job! Everything looks nice now!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 10.0.x-dev » 9.4.x-dev
Category: Feature request » Bug report
Status: Reviewed & tested by the community » Fixed

Committed and pushed 5f9abc4f1c to 10.1.x and 9e0541e4de to 10.0.x. Thanks!
Committed and pushed d0f5680c2f to 9.5.x and 6410b07862 to 9.4.x. Thanks!

Run yarn run build:css and backported to 9.4.x as this is an accessibility bug fix.

  • alexpott committed 5f9abc4 on 10.1.x 
    Issue #3311776 by Chi, Nitin shrivastava, emarinho, ckrina: Claro:...

  • alexpott committed 9e0541e on 10.0.x 
    Issue #3311776 by Chi, Nitin shrivastava, emarinho, ckrina: Claro:...

  • alexpott committed d0f5680 on 9.5.x 
    Issue #3311776 by Chi, Nitin shrivastava, emarinho, ckrina: Claro:...

  • alexpott committed 6410b07 on 9.4.x 
    Issue #3311776 by Chi, Nitin shrivastava, emarinho, ckrina: Claro:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.