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

Investigate potential UI/UX problems generating API keys #7366

Closed
asmecher opened this issue Oct 5, 2021 · 9 comments
Closed

Investigate potential UI/UX problems generating API keys #7366

asmecher opened this issue Oct 5, 2021 · 9 comments
Assignees
Milestone

Comments

@asmecher
Copy link
Member

asmecher commented Oct 5, 2021

Coalition Publica reports frequent problems getting API keys set up.

  • Potentially a lack of feedback when the API key secret is not configured
  • Users may be misusing the "create a new API key" checkbox and accidentally creating a new key just after copying the last one
@asmecher asmecher self-assigned this Oct 5, 2021
@asmecher
Copy link
Member Author

asmecher commented Oct 5, 2021

@ewhanson, you might have some feedback here?

@asmecher asmecher added this to the 3.4 milestone Oct 5, 2021
@NateWr NateWr added the UI/UX label Oct 6, 2021
@NateWr
Copy link
Contributor

NateWr commented Oct 6, 2021

Yeah, I can confirm that the process for generating a key is very confusing. The problem is that it uses checkboxes and a save button to take actions, but these should really be expressed as buttons. It might be harder to fix than it seems, because I think the user profile tabs are tightly coupled with forms.

However, here's a quick mockup of what I think it ought to look like when a key doesn't exist:

create-api-key

When a key does exist:

delete-api-key

That means that we'd be removing the enableApiKey property for now, and just relying on whether a key exists or not. I think it's good to support API key enable/disable, but maybe hold off until we can redesign the API key management stuff more thoroughly?

@NateWr
Copy link
Contributor

NateWr commented Oct 6, 2021

If working on this, it might be easy to roll this recommendation in too: #3635

@ewhanson
Copy link
Collaborator

ewhanson commented Oct 6, 2021

Yes, I'll also echo that the combination of checkboxes and save button seem to lead to confusion.

From my experience, it's not clear from the "changes saved" message that the key has necessarily changed as the same "changes saved" message pops up whether or not a new API key has been generated. I think one way the perception of "invalid keys" can come up is a user generates a new API key, copies it, but then is unsure and will save again, sometimes checking the boxes again. I think the "generate new API key" checkbox becoming unchecked upon successfully saving may also lead to the perception that their changes have been undone or not saved correctly.

@asmecher
Copy link
Member Author

asmecher commented Nov 2, 2021

There's a spec at #5509 for other additions to the API key toolset, most prominently support for multiple API keys. Our current single-API-key-per-user approach promotes bad behaviour that will become more apparent as API key usage expands, like key sharing and the use of "risky" keys (e.g. Journal Manager or Admin keys). If this issue will require broader work to resolve, perhaps that expansion should be considered at the same time.

@NateWr NateWr removed the UI/UX label May 5, 2022
@touhidurabir touhidurabir self-assigned this Aug 18, 2022
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 23, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 23, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 23, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 23, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 23, 2022
@touhidurabir
Copy link
Member

@asmecher
Copy link
Member Author

Reviewed, thanks! #8195 (review)

touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 24, 2022
@touhidurabir
Copy link
Member

@asmecher requested changes are done and also updated the test . updated PRs

pkp-lib --> #8195
ojs --> pkp/ojs#3503

@asmecher
Copy link
Member Author

Thanks, @touhidurabir, this is so much better than the confusing interface it replaces! I've merged the pkp-lib PR, cherry-picked the OJS one to resolve conflicts, and applied the changes to OMP and OPS as well (they went in verbatim).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

5 participants