Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
docs: underline URLs, change contrast in syntax highlighting#8225
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Include next page in comment as I forgot to include it beforehand
Thanks for the PR,@lucas-amberg! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint. |
netlifybot commentedJan 8, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site configuration. |
Btw@lucas-amberg is there a reason you closed#8217 and sent this new one? In case this is useful info: you don't have to delete a PR then re-make it every time you have a new commit 🙂. You can push commits to the same branch. I'd also recommend sending from a branch other than |
The reason I closed it was just so I could restart with a clean fork, I'm sorry for the confusion. Also thank you for the recommendation, I'll make sure to do that in the future. I appreciate the help as I'm really new to this stuff 😅. |
Ooh! Welcome to Git / GitHub / open-source / whatever-combination-of-those-is-new-for-you! 😄🙌 You're actually doing quite well with adhering to standards (the PR is titled well, addresses an accepted+open issue, etc.). This is great! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Progress! 🚀
Uh oh!
There was an error while loading.Please reload this page.
This route would've been a lot easier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Welcome to the open-source world!
Looks good to me :)
| --token-color-symbol: #277c7b; | ||
| --token-color-number: #098658; | ||
| --token-color-keyword: #00f; | ||
| --token-color-function: #569cd6; | ||
| --token-color-function: #2b74b1; | ||
| --token-color-function-variable: #000; | ||
| --token-color-important: #e90; | ||
| --token-color-class-name: #2b91af; | ||
| --token-color-class-name: #237690; | ||
| --token-color-selector: #800000; | ||
| --token-color-regexp: #800000; | ||
| --token-color-property: #d00; | ||
| --token-color-property: #c70000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[Curious]
How did you choose those specific colors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I ran axe devtools to see which colors on each page were below the 4.5:1 threshold and then I usedthis website to check the contrast ratio and modify the current foreground color until it was over that amount. Then I re-ran the devtools to verify the changes were adequate 🙂.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Big +1 onhttps://webaim.org/resources/contrastchecker, great tool!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks cool! Thanks for working on this! 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[tip] Try to avoid fixing several unrelated issues in one PR, even if they are small. This is generally a good practice + it helps maintainers better focus on reviewing fixes for a particular issue.
If two issues are about the same bug/feature, then they should be merged into one issue. Thus, one issue = one PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for iterating with us on this!
Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
Overview
For URL Underlines: Updated CSS to add underlines to URLs and added another rule to remove underlines from the table of contents, next page buttons, edit page button, and menu sidebars along with the social media links in the footer (as they can be distinguished with the external link icon).
For Syntax Highlighting: Checkedaxe Devtools on multiple pages of documentation to find contrast issues in syntax highlighting and adjusted CSS to improve contrast ratio to be >4.5:1 as specified.