Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add Accessiblity & UI Coverage disabled icons #499

Merged
merged 4 commits into from
Nov 13, 2024
Merged

Conversation

emilyrohrbough
Copy link
Member

@emilyrohrbough emilyrohrbough commented Nov 13, 2024

Screenshot 2024-11-13 at 10 35 12 AM

Copy link

vercel bot commented Nov 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
cypress-design ✅ Ready (Inspect) Visit Preview Nov 13, 2024 6:05pm

Copy link

changeset-bot bot commented Nov 13, 2024

🦋 Changeset detected

Latest commit: 997a29f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cypress-design/icon-registry Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@emilyrohrbough
Copy link
Member Author

@emilmilanov / @ryanjwilke Would it be more appropriate to name these disabled instead of skipped? Used skipped because that is what Emil used in the designs.

Copy link

cypress bot commented Nov 13, 2024

cypress-design    Run #2517

Run Properties:  status check passed Passed #2517  •  git commit beb1ccaacd ℹ️: Merge 997a29f6007f923c535f3ed9e468935f57ef767a into 1824a1fcaa8c4afbbaa89a52e35a...
Project cypress-design
Branch Review new-aq-icons
Run status status check passed Passed #2517
Run duration 02m 42s
Commit git commit beb1ccaacd ℹ️: Merge 997a29f6007f923c535f3ed9e468935f57ef767a into 1824a1fcaa8c4afbbaa89a52e35a...
Committer Emily Rohrbough
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 222
View all changes introduced in this branch ↗︎
UI Coverage  11.36%
  Untested elements 166  
  Tested elements 25  
Accessibility  99.37%
  Failed rules  0 critical   1 serious   0 moderate   1 minor
  Failed elements 22  

@emilmilanov
Copy link
Contributor

@emilmilanov / @ryanjwilke Would it be more appropriate to name these disabled instead of skipped? Used skipped because that is what Emil used in the designs.

I'm fine either way.

@emilyrohrbough emilyrohrbough changed the title Add Accessiblity & UI Coverage skipped icons Add Accessiblity & UI Coverage disabled icons Nov 13, 2024
@@ -0,0 +1,7 @@
<svg viewBox="0 0 48 49" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Contributor

Choose a reason for hiding this comment

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

@emilyrohrbough @emilmilanov This icon appears to be 48x49 instead of 48x48. Probably worth fixing that in case it ever needs to be centered properly once it's used.

Copy link
Contributor

Choose a reason for hiding this comment

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

In Figma icon is 48x48. I'm not sure how we ended up with 48x49.

I'm getting the code below when I export svg from Figma. cc @emilyrohrbough

<svg width="48" height="48" viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg"> <rect x="5" y="4.99927" width="38" height="38" rx="19" fill="#D0D2E0" stroke="#1B1E2E" stroke-width="2"/> <path fill-rule="evenodd" clip-rule="evenodd" d="M24 19C25.3807 19 26.5 17.8807 26.5 16.5C26.5 15.1193 25.3807 14 24 14C22.6193 14 21.5 15.1193 21.5 16.5C21.5 17.8807 22.6193 19 24 19ZM17.2425 19.0299C16.7067 18.8959 16.1638 19.2217 16.0298 19.7575C15.8959 20.2933 16.2217 20.8362 16.7575 20.9701L23 22.5308V26.6667L17.95 33.4C17.6186 33.8418 17.7082 34.4686 18.15 34.8C18.5918 35.1314 19.2186 35.0418 19.55 34.6L24 28.6667L28.45 34.6C28.7814 35.0418 29.4082 35.1314 29.85 34.8C30.2918 34.4686 30.3814 33.8418 30.05 33.4L25 26.6667V22.5308L31.2425 20.9701C31.7783 20.8362 32.1041 20.2933 31.9701 19.7575C31.8362 19.2217 31.2933 18.8959 30.7575 19.0299L24 20.7192L17.2425 19.0299Z" fill="#1B1E2E"/> <circle cx="37" cy="37" r="8" fill="white"/> <path d="M43 37.001C43 33.6873 40.3137 31.001 37 31.001C33.6863 31.001 31 33.6873 31 37.001C31 40.3147 33.6863 43.001 37 43.001C40.3137 43.001 43 40.3147 43 37.001Z" fill="#E1E3ED"/> <path d="M32.7412 41.2274L41.2656 32.7814M43 37.001C43 33.6873 40.3137 31.001 37 31.001C33.6863 31.001 31 33.6873 31 37.001C31 40.3147 33.6863 43.001 37 43.001C40.3137 43.001 43 40.3147 43 37.001Z" stroke="#9095AD" stroke-width="2" stroke-linecap="round"/> </svg>

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure, I exported it as well 😅 I'll try it again

Copy link
Member Author

Choose a reason for hiding this comment

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

Weird! Just exported again and have 48x49

@ryanjwilke
Copy link
Contributor

I think using skipped instead of disabled makes sense to me. That said, I'm not entirely sure what it's for. That would probably give me a stronger opinion about the name. Where/when do we expect to use these icons?

@emilmilanov
Copy link
Contributor

@ryanjwilke Icons will be used in empty states only.

@ryanjwilke
Copy link
Contributor

@emilyrohrbough I see. I kind of agree with your assessment then that using the word disabled may make more sense in this case. I thought it was more like a skipped element or something like that, but I see now we're communicating when the UI Coverage & Accessibility haven't been enabled yet in the Cloud, which feels different to me. I would recommend shifting to the "disabled" naming strategy in this case.

@emilyrohrbough
Copy link
Member Author

@ryanjwilke name updates are done!

@emilyrohrbough emilyrohrbough merged commit c1a0110 into main Nov 13, 2024
15 checks passed
@emilyrohrbough emilyrohrbough deleted the new-aq-icons branch November 13, 2024 18:30
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.

3 participants