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

Use window object to generate copy URLs for tasks and organizations #5720

Merged
merged 4 commits into from
Apr 26, 2023

Conversation

HelNershingThapa
Copy link
Contributor

@HelNershingThapa HelNershingThapa commented Apr 19, 2023

This PR does the following:

  • Closes [BUG] Task link copy generates incorrect url #5719 . Use the browser window object instead of the React Router location object to generate task and organization URLs. The values of the location object must have changed, making the origin variable undefined when we migrated to React Router from Reach Router.
  • Removes the dependency on @lokibai/react-use-copy-clipboard previously used for copying text to the clipboard. This is due to deprecated usage of the document.execCommand function and lack of recent commits (last commit over 3 years ago). Instead, the navigator.clipboard.writeText function has been implemented to achieve the desired copy-to-clipboard functionality.

Using window object from the browser instead of the location object from react router to generate URLs for tasks. The values of the location object must have changed making the origin variable undefined when we migrated to React Router from Reach Router
@HelNershingThapa
Copy link
Contributor Author

@ramyaragupathy, I could use some help with testing the functionality of copying the intended URL to the clipboard. We don't currently have code coverage for this feature, and we've been using @lokibai/react-use-copy-clipboard to achieve the copy-to-clipboard functionality. However, I've encountered issues simulating the navigator.clipboard object to ensure that the expected text has been copied to the clipboard but haven't been able to develop a workaround. Can I go ahead and merge this PR for the time being and deal with the test cases later?

@ramyaragupathy
Copy link
Member

@HelNershingThapa - do we know what altered the expected behaviour?

@HelNershingThapa
Copy link
Contributor Author

@HelNershingThapa - do we know what altered the expected behaviour?

Yes, we do. When we switched from Reach Router to React Router, the value of the object we use to build these links changed, which is why.

@HelNershingThapa HelNershingThapa changed the title Use window object to generate copy URLs for tasks Use window object to generate copy URLs for tasks and organizations Apr 19, 2023
@ramyaragupathy ramyaragupathy requested a review from d-rita April 19, 2023 14:27
Remove outdated dependency due to deprecated function usage and lack of recent commits (last commit over 3 years ago).
Also adds text case to check if the writeText function for clipboard API has been called with the expected text
@github-actions github-actions bot added dependencies Pull requests that update a dependency file javascript labels Apr 20, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@d-rita d-rita left a comment

Choose a reason for hiding this comment

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

The functionality works and the tests are good. LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file scope: frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Task link copy generates incorrect url
3 participants