developertools-tech / developertools.tech

A collection of tools for developers served as a PWA
https://www.developertools.tech
MIT License
39 stars 45 forks source link

Feature: Color contrast checker #131

Closed sushma1031 closed 10 months ago

sushma1031 commented 10 months ago

Type of Pull Request

Related Issue #s or links (if any): Fixes #96

Description of Changes

Added a text contrast checking tool in the colors page, similar to the website referenced in the issue. Added the required namespace and tests as well.

netlify[bot] commented 10 months ago

Deploy Preview for developertools-tech ready!

Built without sensitive environment variables

Name Link
Latest commit 9c3254a740ad5a5cfd5758cc49cc1b20041c1028
Latest deploy log https://app.netlify.com/sites/developertools-tech/deploys/653fc66f91e0110008f5fd28
Deploy Preview https://deploy-preview-131--developertools-tech.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

sushma1031 commented 10 months ago

I'll take care of the remaining changes.

dlford commented 10 months ago

@sushma1031 I'm not sure what you mean by "I think the swap button can fit on the left, which text field should I add it to?"? I mainly just wanted the swap button to not be below the other buttons for that field, does that make sense? Apologies for not being more clear.

The input text makes sense, that can stay as is.

Yes, the contrast text can be larger, the current size is perfectly fine, just the semantics of heading tags does not make sense for that.

Thank you!

sushma1031 commented 10 months ago

I'm not sure what you mean by "I think the swap button can fit on the left, which text field should I add it to?"? I mainly just wanted the swap button to not be below the other buttons for that field, does that make sense? Apologies for not being more clear.

Oh, right, I understood what you mean. The swap button isn't actually part of the text field, its purpose is to allow the user to swap the background and foreground colors, so I thought placing it in between the fields would be appropriate. Similar to this: IMG-20231028-WA0000

dlford commented 10 months ago

@sushma1031 I see what you mean, maybe it just needs a little more vertical margin to get it away from the other buttons on that field? Whichever you think is best.

sushma1031 commented 10 months ago

Yes, I agree that a little more vertical margin would be better, I'll add the same.

sushma1031 commented 10 months ago

"Normal Text", "Large Text", and "Graphical Objects and User Interface Components" headings should be h3s, not h5s

image

@dlford I made the mentioned headings h3, and I think they look too large. Can we reduce the font size?

sushma1031 commented 10 months ago

maybe it just needs a little more vertical margin to get it away from the other buttons on that field? Whichever you think is best.

image

Is this much vertical margin sufficient?

dlford commented 10 months ago

@sushma1031 that vertical margin looks good!

For the h3s, yes please make them smaller, thank you!

sushma1031 commented 10 months ago

@dlford I made the requested changes, let me know if there's anything else!

dlford commented 10 months ago

@sushma1031 looks great, thank you!