sul-dlss-deprecated / universalviewer

The Universal Viewer is a community-developed open source project on a mission to help you share your content with the world
http://universalviewer.io
Other
0 stars 1 forks source link

Eliminate modal from authentication process #33

Closed ggeisler closed 6 years ago

ggeisler commented 6 years ago

(Broken out from #24)

The colored information banner at the top of the viewer for logging in is a good first step for authentication, but do we need the subsequent modal windows that appear when the user 1) clicks login, and 2) clicks log in and then Cancel?

Could the login button go straight to the institution auth challenge?

That would eliminate the modal that has the Cancel button, so the subsequent modal would also be eliminated. If the user fails the auth challenge, they could just be returned to the initial state and the user can either try the log in button again or, realizing they don't have the necessary credentials, click the close "X" to remove the information banner.

mejackreed commented 6 years ago

I'm not sure if this is even possible but I have a question out. If it is not possible, we can add text to the modal like this:

screen shot 2017-07-20 at 8 07 41 am

Any thoughts on what should be there?

tomcrane commented 6 years ago

There are two patterns to consider - one where the user can't see anything at all unless they go through an auth process:

http://universalviewer.io/examples/#?c=0&m=0&s=0&cv=0&manifest=https%3A%2F%2Fiiifauth.digtest.co.uk%2Fmanifest%2F01_standard-login

... and one where the user can see something, but the UV is aware that something better may be available:

http://universalviewer.io/examples/#?c=0&m=0&s=0&cv=0&manifest=https%3A%2F%2Fiiifauth.digtest.co.uk%2Fmanifest%2F11_kitty_joyner

In the first, the UV goes straight to the modal because there's nothing else to do. There was never a yellow bar.

In the second, the auth call to action stays out of the way to let the user see the object, but is noticeable enough to invite a further interaction for users who think they might be able to get a better version.

That's the thinking behind the current patterns.

See also @mejackreed's comment: https://github.com/sul-dlss/universalviewer/issues/19#issuecomment-316683744

If that was implemented, the yellow bar would take on the function of the modal (same text, same calls to action) when there's something in the viewport, but you wouldn't have the yellow bar and the modal for the same item, it would be one or the other depending on whether an unauthed user can see anything.

edsilv commented 6 years ago

I've updated the v3 branch of the UV to:

  1. use the labels included in the info.json in the yellow bar instead of getting them from config.
  2. bypass the modal login dialogue when clicking 'login' in the yellow bar.

http://universalviewer.io/examples/#?c=0&m=0&s=0&cv=0&manifest=https%3A%2F%2Fiiifauth.digtest.co.uk%2Fmanifest%2F11_kitty_joyner

To get these changes you'll need to sync your fork and do an npm update manifesto.js and npm update manifold, then rebuild.

tomcrane commented 6 years ago

It bypasses the modal but doesn't take me to the login page, it goes straight to the failure state (shows me the failure message). The popup is being blocked, presumably because the window.open() is not on the same event sequence as the click on login.

(both Firefox and Chrome on Ubuntu)

edsilv commented 6 years ago

That's weird, it's working fine for me... If anything the event is happening more directly than before as the modal is being bypassed:

https://github.com/UniversalViewer/universalviewer/blob/v3/src/modules/uv-shared-module/Auth1.ts#L90

Compare that with line 101

edsilv commented 6 years ago

Ah, the click from the modal confirm button is nearer to the window.open... Too much is happening after clicking the info bar login button for it to be deemed as user initiated.

tomcrane commented 6 years ago

Have you at some point enabled popups for the uv domain? Once you've done that you won't see the problem, but obviously we don't want to require that users allow popups.

edsilv commented 6 years ago

Yep, popups are enabled for the uv domain. Disabling gives me the same error.

tomcrane commented 6 years ago

We've avoided requiring users to enable popups so far, and I feel strongly that we should not require this ever. The spec limits itself to only require an action that is permitted by current browsers without enabling popups (the window.open() for the external auth login), but browsers will treat that as a popup and block if it isn't directly triggered by a user action. This browser strictness about what is a real user action can throw a spanner into what would otherwise be elegant asynch flow. Is there an extra callback happening in the click on the yellow bar login button, so that by the time it gets to window.open it's no longer considered to be on the same event loop? Is there a callback between the click on the link and line 85? If it works for the modal it must be doable for the yellow bar somehow.

edsilv commented 6 years ago

See https://stackoverflow.com/a/25050893

I think this could work where the login button in the yellow bar opens a "holding page" and passes a reference to it through the auth chain. When it reaches https://github.com/UniversalViewer/universalviewer/blob/v3/src/modules/uv-shared-module/Auth1.ts#L86 it looks for a passed window object and redirects that instead of opening a new one.

mejackreed commented 6 years ago

@edsilv looking into this more today. We seem to be having issues in IE when embedding UV, referencing authed objects from within an iframe.

edsilv commented 6 years ago

@mejackreed Try pulling latest on v3. This should be fixed now. Have added an authHoldingPage property to ExternalResource. This is immediately set to a _blank page when clicking on the info bar's login button, then redirected to in Auth1.getContentProviderInteraction.

aeschylus commented 6 years ago

Okay. We've updated all the relevant themes and dependencies to test v3 in our purl and exhibits platforms. We're still getting the following modals showing up:

PURL

screen shot 2017-09-29 at 13 21 02

Exhibits

screen shot 2017-09-29 at 13 21 02

Further information about the exhibits bug is here: https://github.com/sul-dlss/sul-embed/issues/827

hackartisan commented 6 years ago

sul-dlss#33

Estimate: 2 days

Caveat: Will need to discuss further with Stanford team, probably another skype call, before attempting a fix. Added by edsilv

mejackreed commented 6 years ago

Manifest we are seeing issues with: https://purl.stanford.edu/ry222mj7473/iiif/manifest

edsilv commented 6 years ago

Closing as fixed