Skip to content

refactor(site): clean up clipboard functionality and define tests #12296

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

Merged
merged 19 commits into from
Feb 28, 2024

Conversation

Parkreiner
Copy link
Member

@Parkreiner Parkreiner commented Feb 25, 2024

Closes #12171 and makes more progress towards completing #11421

Changes made

  • Refactors the current useClipboard functionality for better clarity
    • Also renamed one of the returned properties and added a JSDoc comment to help communicate some of the limitations around it
  • Adds functionality for notifying the user of a failed clipboard copy
    • By default, this goes through the global snackbar, but it can be overridden if need be
  • Added tests for useClipboard, both for HTTPS and HTTP-only connections
  • Updated the code for our CodeExample component to ensure that clicking the component doesn't cause the inner button to double-fire
  • Figured out a way to add back the anti-flicker styling for the HTTP workaround that got removed by a previous PR

Notes

  • The testing setup for this PR is very...unconventional. I would recommend reading useClipboard.test-setup.ts to get a overview of why things are set up the way they are
  • To be on the safe side, in addition to the automated tests, I also tested the HTTP-only functionality on Chrome, Firefox, and Safari. Didn't notice any issues with any of them
  • Once this PR is through, I am planning to update the code to use Clipboard.js, using the tests to ensure there aren't any regressions
  • Also still planning to figure out how to update the Firefox visual bug where the CLI auth panel isn't fully centered
    • Need to look into Emotion docs to see what options there are for declaring multiple browser-specific values for the same style property

@Parkreiner Parkreiner changed the title chore: add tests for useClipboard chore(site): add tests for useClipboard for HTTPS and HTTP connections Feb 26, 2024
@Parkreiner Parkreiner changed the title chore(site): add tests for useClipboard for HTTPS and HTTP connections refactor(site): clean up clipboard functionality and define tests Feb 26, 2024
@Parkreiner Parkreiner requested review from a team and code-asher and removed request for a team February 26, 2024 14:01
@Parkreiner Parkreiner marked this pull request as ready for review February 26, 2024 14:01
Comment on lines +74 to +75
* It feels silly that you have to make a whole dummy input just to simulate a
* clipboard, but that's really the recommended approach for older browsers.
Copy link
Member

Choose a reason for hiding this comment

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

I remember having to do similar things a long time ago, traumatic memories. 😆

@Parkreiner Parkreiner merged commit 087f973 into main Feb 28, 2024
@Parkreiner Parkreiner deleted the mes/clipboard-tests branch February 28, 2024 02:05
@github-actions github-actions bot locked and limited conversation to collaborators Feb 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to copy session token from the cli-auth page
2 participants