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

chore: refactor copy and share components for better accessibility 🪄✨ #1492

Closed
wants to merge 11 commits into from
Closed

chore: refactor copy and share components for better accessibility 🪄✨ #1492

wants to merge 11 commits into from

Conversation

0xabdulkhaliq
Copy link
Contributor

@0xabdulkhaliq 0xabdulkhaliq commented Aug 4, 2023

Fixes Issue

Closes #1479

Changes proposed

  • This PR introduces several significant enhancements to the CopyToClipboard.tsx and Share.tsx files, focusing on code refactoring and accessibility improvements. The key changes are as follows:

    • Code Refactoring: Both CopyToClipboard.tsx and Share.tsx components have undergone code refactoring to remove unnecessary elements, such as wrapping divs, and improve code readability and maintainability.

    • Improved Accessibility: The accessibility of the components has been greatly enhanced. We replaced the title attributes with more descriptive and accessible aria-label attributes for the icons. Additionally, we introduced aria-live="polite" to announce changes to screen readers in a polite manner.

    • Styling Enhancement: The inline styles in the components have been removed, and we now rely on CSS classes for styling. This makes the code more organized and easier to manage.

    • Reduced Bloatware: We have trimmed unnecessary elements and styles, resulting in a leaner and more efficient codebase.

    • Icon Change on Success: When the copy action is successful, the copy icon now changes to a checkmark icon (FaCheckSquare). This gives users clear feedback on the success of the copy operation.

    • Resetting Copied Icon: The copied icon now resets back to the original copy icon (FaRegCopy) after 2 seconds, ensuring a smoother user experience.

    • With these improvements, the CopyToClipboard and Share components are now more accessible, efficient, and user-friendly. The PR contributes to the overall enhancement of the repository's codebase and user experience.

Demonstrating Video

Capture_2023-08-04-21-34-43.mp4

[ This video will not demonstrates the accessibility feature, but it can demonstrate other changes made in this PR ]

[ My device didn't allow to record the internal audio during recording, So that i can't demonstrate the accessibility feature that announcing Copy Link and Link Copied! after copying the link. Sorry for the inconvenience.]

@vercel
Copy link

vercel bot commented Aug 4, 2023

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

Name Status Preview Comments Updated (UTC)
linkshub ❌ Failed (Inspect) Aug 8, 2023 8:47am

@github-actions github-actions bot added accessibility Code is not up to WCAG standards quick-fix Shouldn't take much time to finish goal: refactor Refactoring the codebase labels Aug 4, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you, 0xabdulkhalid, for creating this pull request and contributing to LinksHub! 💗

The maintainers will review this Pull Request and provide feedback as soon as possible! 😇
We appreciate your patience and contribution, Keep up the great work! 😀

@0xabdulkhaliq
Copy link
Contributor Author

Hi there @rupali-codes, @CBID2 👋🏻

I hope you're doing well. I wanted to let you know about the PR I've created for the LinksHub. Initially, I started with the intention of removing unwanted elements during the code refactoring process. However, my passion for accessibility and best practices drove me to dive deeper into enhancing both the Copy and Share components.

The changes made in this PR bring significant improvements in terms of accessibility, making the components more inclusive and user-friendly. Additionally, I've streamlined the code, removing unnecessary elements and optimizing the overall performance.

I believe these updates will greatly contribute to the core functionality and usability of the project. It would be amazing if you could review and consider merging this PR. Also, to support my work, I'd appreciate it if you could add the level3 labels to recognize the impact of these enhancements.

Please let me know your thoughts, and I'm open to any feedback or suggestions.

Thank you for your time and consideration.

@CBID2 CBID2 added gssoc GirlScript Summer of Code participants level3 Making completely new feature labels Aug 4, 2023
@CBID2 CBID2 requested a review from k-deepak04 August 4, 2023 16:36
@k-deepak04
Copy link
Contributor

@0xabdulkhalid you changed quite few behaviour , like hover effect also the pop-up that use to come on copying the link from clipboard that.

bandicam.2023-08-04.22-48-23-592.mp4

@CBID2 @rupali-codes please look into this

@CBID2
Copy link
Collaborator

CBID2 commented Aug 4, 2023

Visit Preview

Ooh you're right @k-deepak04. Not having the hover effect can make it difficult for readers, especially for those who are low-vision, to know when to copy the link @0xabdulkhalid

@0xabdulkhaliq
Copy link
Contributor Author

0xabdulkhaliq commented Aug 5, 2023

@k-deepak04, @CBID2

You're saying about this Purple Colored Tooltip right ? But you can see that it provided Two Clues at once.

One is explicitly designed with p element and another one is title. The important thing is the p is not accessible to accessibility devices but the title is completely accessible.

Another thing is we don't need the p element, It could look nice as a purple colored tooltip but in reality it's just a bloatware.

You could experience how both tooltip's are layered on top of each other, Hope you understood.

image

There's already title is present to do that Job, We don't want explicit p element to behave like title.

The title is enough to provide visual clue to the user who have low vision.

You can look this demonstration 👀

Capture_2023-08-05-08-32-14.mp4

Copy link
Owner

@rupali-codes rupali-codes left a comment

Choose a reason for hiding this comment

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

@0xabdulkhalid have you removed that tooltip?

@0xabdulkhaliq
Copy link
Contributor Author

0xabdulkhaliq commented Aug 5, 2023

@rupali-codes Yeah, I removed the Tooltip.

Instead i changed icon, So if the user successfully copied the link then the icon with change to success and then after 2 seconds it will revert to normal state (copy icon).

Capture_2023-08-05-12-26-06.mp4

@rupali-codes
Copy link
Owner

@rupali-codes Yeah, I removed the Tooltip.

Instead i changed icon, So if the user successfully copied the link then the icon with change to success and then after 2 seconds it will revert to normal state (copy icon).

You could investigate the demonstrating videos above.

But its looking kinda confusing, could you please add the tooltip while changing the icon as well? that would be more helpful. And even you can use React-tooltip if you want to, or just add as it was before :)

Thanks

@0xabdulkhaliq
Copy link
Contributor Author

0xabdulkhaliq commented Aug 5, 2023

@rupali-codes as you wish, I will make the tooltip to be viewed once the user copied the link. It will say Copied!

Wait, there's a small bug i need to fix now. I will update after fixing that!

@rupali-codes
Copy link
Owner

@0xabdulkhalid could you just make it Copied? also Copy on hover :)

@0xabdulkhaliq
Copy link
Contributor Author

Done as you requested @rupali-codes 👍🏻

Capture_2023-08-05-13-22-34.mp4

@rupali-codes
Copy link
Owner

its kinda cutting, could u add some margin to it?? then we are good to go

image

@0xabdulkhaliq
Copy link
Contributor Author

Is this ok @rupali-codes ?

Capture_2023-08-05-13-44-53.mp4

@CBID2
Copy link
Collaborator

CBID2 commented Aug 5, 2023

Is this ok @rupali-codes ?

Capture_2023-08-05-13-44-53.mp4

Looks better now in my opinion @rupali-codes

Copy link
Collaborator

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Ignore my previous comment @0xabdulkhalid. For some odd reason, the deployment failed. @rupali-codes, can you fix this?

@0xabdulkhaliq
Copy link
Contributor Author

@CBID2 I think the react-tooltip has not installed.

I tried to install that on local, but it doesn't. There's some conflicts on packages.

Copy link
Contributor

@k-deepak04 k-deepak04 left a comment

Choose a reason for hiding this comment

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

check, deploymnet is failing

@0xabdulkhaliq
Copy link
Contributor Author

Guys @CBID2, @k-deepak04 I'm closing this.

For some reason i can't install react-tooltip on this branch, So i created a new branch a new pull.

Here's the new Pull #1517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Code is not up to WCAG standards goal: refactor Refactoring the codebase gssoc GirlScript Summer of Code participants level3 Making completely new feature quick-fix Shouldn't take much time to finish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Refactor]: Share and Copy Button Components for Improved Accessibility ⚒️✨
4 participants