#1725 Fix non-obvious flow on new token page
Merged by pingou. Opened by cep.
cep/pagure fix_non_obvious_new_token_page  into  master

Download 1725.patch

Improve the Create new token page by adding info, displaying error messages, and other minor changes.

Fixes https://pagure.io/pagure/issue/1571

This checks off all the items in the checklist on the issue page of #1571. But I was wondering if we can keep this open for a few days and add tests for this as well?

history.back() wasn't working?

missing space?

Most definitely up for waiting and get tests for this :)

Nope. This is because when an empty form submits, history.back() is simply the same page. It wouldn't work in case the user visited the URL directly as well.

Showing the settings page on clicking 'Cancel' is probably non-intuitive. What do you propose?

You mean "at least"?

I think it's fine to go back to the settings page since that's where they come from

That's indeed what I had in mind :)

I believe if an an empty form submits, history.back() is not the same page :)

Will fix this :)

It seems to be. Can you please confirm?

Steps to reporduce:
- Visit the Settings page of a project.
- Click on 'Create new key' under 'API keys'.
- Click 'Add' without selecting any permissions.
- After page reloads, click 'Cancel'. Same page will be displayed.
- Click 'Cancel' again. You'll be taken to the Settings page.

@cep, As soon as you press Cancel, you will be
taken back to the Settings page :)

@rahulbajaj, I couldn't get it to go to the settings page on the first click, in Google Chrome or Firefox. Did you try following the steps above?

In case this is an issue specific to me, can someone else test and verify?

@cep, I am using Firefox and it works well for me :)

FWIW, I am seeing the same behavior as @cep:

  • Go to settings
  • Click on Create a new key
  • Click on Add without checking any of the checkboxes
  • Page reloads to the same page
  • Click on Cancel
  • Page reloads to the same page
  • Click on Cancel a second time
  • Page reloads to Settings

And it does make sense :)

Anyway, since the only way to access this page is via the project's settings page, I think it's fine to just replace this bit of JS with a link to that settings page.

Alright, so imho, let's fix this atleast, rebase and merge :)

rebased

Okay, done! :)

I'm just getting started reading up on tests. So I'll open a separate PR for it later.

Ok that's cool, let's open a ticket so we can assign it to you and not forget about it :)

Pull-Request has been merged by pingou

Metadata