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

Fixed extra returns copied with api token on account pg [ref #7601] #7603

Merged
merged 1 commit into from Feb 17, 2021

Conversation

mheppler
Copy link
Contributor

@mheppler mheppler commented Feb 10, 2021

What this PR does / why we need it:

Fixes odd bug in Firefox where extra returns were added before and after the api token when copied to the clipboard. Also fixes the copy function success from trying to add an "OK" success icon to the copy button.

Which issue(s) this PR closes:

Closes #7601 API Token - Copy to clipboard REDUX

Related to #6039 Copy buttons for API Token and Private URL

Special notes for your reviewer:

To resolve this, rather have the plugin get the token from the code element on the pg, I am passing the token directly to the clipboard with a data attribute data-clipboard-text="#{ApiTokenPage.apiToken}" on the copy button.

Suggestions on how to test this:

Make sure you're only getting the single string of the api token from your clipboard, with no additional returns before or aftere. Phil was using the terminal commandline to prove this. Something pasting in a WYSIWYG that formats the text will not show you these odd invisible returns.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I didn't test this but the code change makes sense.

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🦁 to QA 🔎✅ Feb 10, 2021
@kcondon kcondon self-assigned this Feb 11, 2021
@kcondon
Copy link
Contributor

kcondon commented Feb 16, 2021

This works but I'm finding page refresh sets tab to account info in FF, stays on api tab on chrome . Doesn't happen on demo, stays on api tab. Note that on develop it refreshes to My Data, there was recently a discussion/fix/change to this page that made my data tab the default, see Stephen or Gustavo.

@mheppler
Copy link
Contributor Author

Pretty sure nothing in this PR would effect the selected tab state on page refresh. Since there was work recently done to this exact issue in #7534 4d430c2, I would suggest this PR fixing an entirely different bug not get held up by those changes, and any additional issues with the default tab state gets a new issue.

@mheppler mheppler removed their assignment Feb 16, 2021
@kcondon
Copy link
Contributor

kcondon commented Feb 17, 2021

@mheppler Were you able to reproduce the behavior though? I don't want to merge something that alters develop. I'll try reproducing it, maybe it was a bad testing day ;) Also asked Stephen for advice. Stephen did not see anything weird and could not reproduce it. Thanks @sekmiller . Will rebuild and retest once Jenkins is back online. OK, was able to retest and am not seeing weirdness.

@kcondon kcondon moved this from QA 🔎✅ to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Feb 17, 2021
@kcondon kcondon assigned mheppler and kcondon and unassigned mheppler Feb 17, 2021
@kcondon kcondon moved this from IQSS Team - In Progress 💻 to QA 🔎✅ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Feb 17, 2021
@kcondon kcondon merged commit a82b7c3 into develop Feb 17, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Feb 17, 2021
@kcondon kcondon deleted the 7601-api-token-clipboard-fix branch February 17, 2021 15:09
@djbrooke djbrooke added this to the 5.4 milestone Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

API Token - Copy to clipboard REDUX
4 participants