BioDrop icon indicating copy to clipboard operation
BioDrop copied to clipboard

Published 20 hours ago • verified publisher icon EddieHubCommunity

Make links more accessible with custom link component (#2523)

Open clintonlunn opened this issue 2 years ago • 3 comments

Fixes Issue #2523

Changes proposed

I added a custom Link component that would give us a little bit better control over some of the accessibility concerns. I added tailwind classes to add text decoration and hover actions. I included these changes in both js and mdx files.

I also fixed broken implementation of ClipboardCopy in tags.mdx

Check List (Check all the applicable boxes)

  • [x] My code follows the code style of this project.
  • [ ] My change requires changes to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] All new and existing tests passed.
  • [x] This PR does not contain plagiarized content.
  • [x] The title of my pull request is a short description of the requested changes.

Screenshots

First screenshot to show without hover, second screenshot shows a link being hovered over (pointer invisible, sorry). image image

Note to reviewers

clintonlunn avatar Jan 12 '23 09:01 clintonlunn

Welcome @clintonlunn! Thank you so much for your first pull request!

github-actions[bot] avatar Jan 12 '23 09:01 github-actions[bot]

@eddiejaoude Thanks for the review! I believe I resolved all of the issues you pointed out.

However, I found some more occurrences of using next/link Link component that could be swapped out also (i.e. Footer.js and some other places). If you agree with the direction of using a Link component wrapper around the next/link, do you want me to change the links out everywhere?

clintonlunn avatar Jan 13 '23 23:01 clintonlunn

If you agree with the direction of using a Link component wrapper around the next/link, do you want me to change the links out everywhere?

Thank you for making the changes. I will review tomorrow (Sunday), but yes we want everywhere to be consistent, if you could do that, that would be great 👍

eddiejaoude avatar Jan 14 '23 23:01 eddiejaoude

Thank you for your contribution. More conflicts are appearing, so I will merge to a temporary branch and try to resolve

eddiejaoude avatar Jan 23 '23 21:01 eddiejaoude

@eddiejaoude apologies for the delay. I got caught up in work things. I made the changes locally and then started having issues bringing my fork up to date to resolve conflicts. I can resolve the merge conflicts on my end still if you want.

clintonlunn avatar Jan 23 '23 21:01 clintonlunn

No problem @clintonlunn, I know people are busy, I resolved conflicts already in this PR https://github.com/EddieHubCommunity/LinkFree/pull/3866 - if you have more files to commit, can you create a PR to the links branch instead of main and I can get it merged

eddiejaoude avatar Jan 23 '23 21:01 eddiejaoude