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.

Sign up for GitHub

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

Jump to bottom

Add API key field and copy to clipboard logic on settings page #2012

Merged
merged 2 commits into from
May 31, 2022
Merged

Add API key field and copy to clipboard logic on settings page #2012

merged 2 commits into from
May 31, 2022

Conversation

maxxcrawford
Copy link
Contributor

@maxxcrawford maxxcrawford commented May 27, 2022 •
edited
Loading

Fixes: MPP-2030

Notes:

  • I plan on writing a test. I found this SO article to get me started. We also don't have one for the Alias page, so I'll add that as a chore.

Testing steps:

  • Log in
  • Go to /profile/settings page
  • Expected: There should be a new row under "Privacy" called "API Key"
  • Click the copy icon
  • Expected: API key should be copied to clipboard

Screenshots:

Screenshot 2022-05-27 at 16-26-59 Firefox Relay

Screenshot 2022-05-27 at 16-26-51 Firefox Relay

@maxxcrawford maxxcrawford requested review from lloan and Vinnl May 27, 2022 21:22
@netlify
Copy link

netlify bot commented May 27, 2022 •
edited
Loading

❌ Deploy Preview for fx-relay-demo failed.

Name Link
🔨 Latest commit 8e41a0f
🔍 Latest deploy log https://app.netlify.com/sites/fx-relay-demo/deploys/62914378797842000817b5d4

lloan
lloan suggested changes May 27, 2022
View reviewed changes
Copy link
Contributor

@lloan lloan left a comment

Choose a reason for hiding this comment

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

Pretty straightforward - awesome new feature.

frontend/src/pages/accounts/settings.page.tsx Outdated Show resolved Hide resolved
frontend/src/pages/accounts/settings.page.tsx Outdated Show resolved Hide resolved
frontend/src/pages/accounts/settings.page.tsx Outdated Show resolved Hide resolved
frontend/src/pages/accounts/settings.page.tsx Show resolved Hide resolved
maxxcrawford and others added 2 commits May 30, 2022 14:26
@maxxcrawford @Vinnl
Add API key field and copy to clipboard logic
0be947c 
Co-authored-by: Vincent <[email protected]>
@Vinnl
Make the "Copied!" notification visible again
e57aefc 
It was made invisible in 2f07551,
because the notification was located outside the `.copy-controls`,
and anything outside it was hidden to prevent long mask addresses
from breaking the layout.

Thus, with this change, I've simply moved the notification to
overlap the copy button and be positioned in the middle of it.
@Vinnl Vinnl force-pushed the settings-api-key branch from d2c5a4c to e57aefc Compare May 30, 2022 12:26
@Vinnl Vinnl requested a review from lloan May 30, 2022 12:35
Vinnl
Vinnl approved these changes May 30, 2022
View reviewed changes
Copy link
Collaborator

@Vinnl Vinnl left a comment

Choose a reason for hiding this comment

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

This is ready to go as far as I'm concerned. I have a vague recollection of needing to merge this today, but since it's not really QA'able other than making sure that it's actually shown and copied to the clipboard, I'll hold off until US folks are back.

lloan
lloan approved these changes May 31, 2022
View reviewed changes
Copy link
Contributor

@lloan lloan left a comment

Choose a reason for hiding this comment

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

Looks good!

@lloan lloan merged commit fa45d73 into main May 31, 2022
@maxxcrawford maxxcrawford deleted the settings-api-key branch June 10, 2022 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lloan lloan lloan approved these changes

@Vinnl Vinnl Vinnl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maxxcrawford @Vinnl @lloan