Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

docs(badge): updated non semantics docs for mod override #4162

Merged
merged 4 commits into from
Mar 11, 2024

Conversation

Rajdeepc
Copy link
Contributor

@Rajdeepc Rajdeepc commented Mar 8, 2024

Description

Updated the --mod-badge-background-color-default override for seafoam,indigo,purple,fuchsia,magenta,yellow non semantic variants property which should be

--mod-badge-background-color-seafoam
--mod-badge-background-color-indigo
--mod-badge-background-color-purple
--mod-badge-background-color-fuchsia
--mod-badge-background-color-magenta
--mod-badge-background-color-yellow

Related issue(s)

Motivation and context

How has this been tested?

  • Test case 1
    1. Go here
    2. Do this
  • Test case 2
    1. Go here
    2. Do this

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

Copy link

github-actions bot commented Mar 8, 2024

Lighthouse scores

Category Latest (report) Main (report) Branch (report)
Performance 0.97 0.97 0.98
Accessibility 1 1 1
Best Practices 1 1 1
SEO 1 0.92 0.92
PWA 1 1 1
What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category Latest Main Branch
Total 240.883 kB 228.742 kB 228.463 kB 🏆
Scripts 60.011 kB 54.454 kB 54.432 kB 🏆
Stylesheet 48.351 kB 42.406 kB 42.152 kB 🏆
Document 5.809 kB 5.17 kB 5.167 kB 🏆
Third Party 126.712 kB 126.712 kB 126.712 kB

Request Count

Category Latest Main Branch
Total 43 43 43
Scripts 35 35 35
Stylesheet 5 5 5
Document 1 1 1
Third Party 2 2 2

Copy link

github-actions bot commented Mar 8, 2024

Tachometer results

Chrome

badge permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 397 kB 33.29ms - 34.64ms - unsure 🔍
-3% - +2%
-0.93ms - +0.58ms
branch 388 kB 33.80ms - 34.49ms unsure 🔍
-2% - +3%
-0.58ms - +0.93ms
-
Firefox

badge permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 397 kB 72.89ms - 77.07ms - unsure 🔍
-8% - +1%
-6.35ms - +0.71ms
branch 388 kB 74.95ms - 80.65ms unsure 🔍
-1% - +9%
-0.71ms - +6.35ms
-

@Westbrook
Copy link
Contributor

Interesting. I wish I could keep the historical trajectory of every component in my mind, but alas here is one that I can't. I can say how we got to where we are now, but it seems that being these variants are "official" that we don't need those customizations at all to achieve the colors desired for those Badges. Could we remove them instead? Also, it seem that the same content in the Storybook could use the same.

Related, it seems that there are about 9 variants that we aren't supporting here: https://github.com/adobe/spectrum-web-components/blob/main/packages/badge/src/spectrum-badge.css#L269-L274 Would you be able to include those in the CSS processing and the various pieces of documentation?

@Rajdeepc Rajdeepc requested a review from Westbrook March 11, 2024 05:50
Copy link
Contributor

@Westbrook Westbrook left a comment

Choose a reason for hiding this comment

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

LGTM. Catching up main and then let’s merge this in!

@Westbrook Westbrook force-pushed the fix/badge-non-semantic branch from 4a431b6 to 68619e3 Compare March 11, 2024 11:03
@Westbrook Westbrook merged commit 19e1a49 into main Mar 11, 2024
47 of 49 checks passed
@Westbrook Westbrook deleted the fix/badge-non-semantic branch March 11, 2024 11:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants