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

Fix incorrect target for hidden copy component #2205

Merged
merged 2 commits into from
Jan 29, 2025
Merged

Conversation

hudson-newey
Copy link
Member

@hudson-newey hudson-newey commented Jan 29, 2025

Fix incorrect target for hidden copy component

There was previously a bug where the touch target for the HiddenCopyComponent was the copy icon, not the copy button.

This is was exacerbated by the fact that there were no unit tests to assert that this component would copy text correctly.

Changes

  • Changes the HiddenCopyCompont's copy touch target to the copy button instead of the copy icon
  • Adds unit tests to assert that the copy button works correctly

Issues

Fixes: #2146

Visual Changes

image

Should be unchanged, let me know if you see any discrepancies

Final Checklist

  • Assign reviewers if you have permission
  • Assign labels if you have permission
  • Link issues related to PR
  • Ensure project linter is not producing any warnings (npm run lint)
  • Ensure build is passing on all browsers (npm run test:all)
  • Ensure CI build is passing
  • Ensure docker container is passing (docs)

@hudson-newey hudson-newey added bug Something isn't working ui feature A new feature/page to add to the project triage:medium Medium priority issue or pull request labels Jan 29, 2025
@hudson-newey hudson-newey self-assigned this Jan 29, 2025
@hudson-newey hudson-newey requested a review from atruskie January 29, 2025 01:19
Copy link
Contributor

Size Change: +71 B (0%)

Total Size: 3.91 MB

Filename Size Change
dist/workbench-client/browser/index.html 4.81 kB +1 B (+0.02%)
dist/workbench-client/browser/main-YWRP5WNH.js 0 B -1.13 MB (removed) 🏆
dist/workbench-client/server/main.js 1.94 MB +38 B (0%)
dist/workbench-client/browser/main-YERRD7N3.js 1.13 MB +1.13 MB (new file) 🆕
ℹ️ View Unchanged
Filename Size
dist/workbench-client/browser/assets/buffer-builder-processor-BhnxGUx8.js 1.16 kB
dist/workbench-client/browser/assets/environment.json 555 B
dist/workbench-client/browser/assets/high-accuracy-time-processor-BevUJNwo.js 354 B
dist/workbench-client/browser/assets/test-assets/index.html 21 B
dist/workbench-client/browser/chunk-NPDQGTTA.js 1.17 kB
dist/workbench-client/browser/chunk-U5EWAFH4.js 379 kB
dist/workbench-client/browser/manifest.json 147 B
dist/workbench-client/browser/polyfills-C5CKP5CH.js 12.4 kB
dist/workbench-client/browser/styles-ETV6J7SM.css 39.6 kB
dist/workbench-client/server/267.js 390 kB
dist/workbench-client/server/327.js 4.21 kB

compressed-size-action

Copy link
Contributor

Unit Test Results

         6 files           6 suites   10m 13s ⏱️
24 246 tests 23 502 ✔️ 744 💤 0
24 468 runs  23 724 ✔️ 744 💤 0

Results for commit 1078ecc.

@hudson-newey hudson-newey merged commit 784fbf4 into master Jan 29, 2025
14 checks passed
@hudson-newey hudson-newey deleted the copy-button-fix branch January 29, 2025 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage:medium Medium priority issue or pull request ui feature A new feature/page to add to the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Touch target for 'Authentication Token' copy button does not match button size
2 participants