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

Suggestion for further re-work of "Add/Update OIDC client" page #1262

Closed
aliaksander-samuseu opened this Issue Sep 25, 2018 · 15 comments

Comments

Projects
None yet
4 participants
@aliaksander-samuseu

aliaksander-samuseu commented Sep 25, 2018

The look of the page is sure much better now, but, IMO, controls which are rarely used still take too much space for themselves, while pushing the most often changed settings to the bottom and cluttering the page. On the attached picture I tried to suggest an even more compact an easy-to-grasp way of placing them on the page, when the least used settings are either pushed down the page, or moved to a hidden (folded) lists or to pop-up windows, triggered by user clicking a button.
oidc_client_new_layout

@aliaksander-samuseu

This comment has been minimized.

aliaksander-samuseu commented Sep 25, 2018

One more small suggestion in addition to what picture shows. It seems now when you create a client registration manually, its expiration is set to 24 hours. Usually when we add a client in such way, we don't want it to expire at all. In previous versions it was default behaviour for manual addition, and I think it should stay such, as it will require user to do extra job of setting it to never expire each time they create a client in web UI otherwise.

@aliaksander-samuseu

This comment has been minimized.

aliaksander-samuseu commented Sep 25, 2018

We also should try to come up with a more smart placement of the "Add"/"Update"/"Cancel" button. Currently, if user arrives at this page to tweak a single setting at the top of it, he needs to scroll all the way down to apply the change. Perhaps the script on the page could detect that a change did take place, and draw a panel with "Update"/"Cancel" button right at the bottom of the current working screen, which would stay there while user scrolls up/down?

This is already more a "quality of life"-type of enhancement, still worth considering.

@willow9886

This comment has been minimized.

Contributor

willow9886 commented Sep 26, 2018

Maybe we should have a button for "Advanced Settings" , which exposes infrequently used configuration options

@aliaksander-samuseu

This comment has been minimized.

aliaksander-samuseu commented Oct 1, 2018

@willow9886
The fields I moved to "General info" on the picture probably won't fit such category very well. They are not that advanced, but still not frequently used (I would also move the "Add Contacts" button there as well, while returning "Client description" text box back to the main page, as it's where it belongs, user must be able to see it first-first when they open this page, as it usually will give them general description of the client's role). Having 2 different categories for those would reduce cluttering, I think.

Also instead of pop-ups triggered by buttons we could split the page into several tabs, like "Standard settings" / "Advanced settings" or "Encryption/Signing algorithms" / "General info".

Or "General info" could be a button on the "Standard settings" tab (as I believe it will be the least used set of fields, no need to dedicate a full tab for it perhaps). For convenience of viewing it, all text assigned to those fields could be displayed on a small pop-up iframe when user hovers mouse's cursor over the button, or some pictogram near it. So they won't need to click it summoning the full diaglog each time they just want to check it.

@aliaksander-samuseu

This comment has been minimized.

aliaksander-samuseu commented Oct 1, 2018

I would probably go with 3 tabs (as we already use tab-based approach on several other core pages):

  1. "Standard settings" or "Basic configuration" - the most important/mandatory properties you need for all/most of clients. "General info" button is placed on this page as well and behaves as described above.

  2. "Advanced settings" - Some less often used, and mostly not critical, but still useful settings, either defined in OIDC specs, or controlling our custom features

  3. "Encryption/Signing algorithms" - worth of dedicating a separate tab for them, as though they are not frequently tweaked manually, yet still there are a lot of them (so they will clutter page if mixed with any other settings) and they may be very important in specific cases or for some setups.

@syntrydy

This comment has been minimized.

Contributor

syntrydy commented Oct 8, 2018

image

@syntrydy

This comment has been minimized.

Contributor

syntrydy commented Oct 8, 2018

image

@syntrydy

This comment has been minimized.

Contributor

syntrydy commented Oct 8, 2018

image

image

@syntrydy

This comment has been minimized.

Contributor

syntrydy commented Oct 8, 2018

@willow9886 @aliaksander-samuseu please check if it is correct.

syntrydy added a commit that referenced this issue Oct 8, 2018

Merge pull request #1298 from /issues/1262
Fix Suggestion for further re-work for add/update client page #1262
@willow9886

This comment has been minimized.

Contributor

willow9886 commented Oct 8, 2018

this looks excellent @syntrydy !

@aliaksander-samuseu

This comment has been minimized.

aliaksander-samuseu commented Oct 9, 2018

@syntrydy @willow9886

Here are some suggestions per each tab:

Sign/Enc properties:

(See picture attached below)

  1. We need to maintain grouping of controls related to a specific item, as it was done on the old page, to enhance usability. It will usually be 3 controls per item, in next fixed order: "JWS alg Algorithm for signing...", "JWE alg Algorithm for encrypting..." and finally "JWE enc Algorithm for encrypting..." On the attached picture only 1 out of 3 groups was preserved (marked with green), groups marked with yellow and orange got separated. Also, order of elements inside each group doesn't match the order mentioned above.

  2. Not sure why "Default Max auth age" (marked red) is placed here, as I don't see obvious relation to enc/signing here. It should be moved to "Advanced settings", I believe
    new_enc-sign_props_page

@aliaksander-samuseu

This comment has been minimized.

aliaksander-samuseu commented Oct 9, 2018

Advanced settings

  1. Fields like "Logo URI", "Policy URI" etc don't look like they belong here, being neither important, nor advanced, yet they consume space right in the center of the page. Most of the time those fields will be left empty anyway. At the very least, we should move them to the bottom of the page, but better still try to find a better place for them (the previous suggestion for them moved to a pop-up window called with a button on the first tab still holds)

  2. The picture seems to be truncated, so I can't see elements further down the page

Standard settings

  1. "Disabled" button should really be moved somewhere up. The idea here is that user should be able to assess the general state of this entity with one quick glance when they open its page, and whether client's entry is disabled or not is pretty important info.
@aliaksander-samuseu

This comment has been minimized.

aliaksander-samuseu commented Oct 9, 2018

Does this "Disabled" control have to be that big, at all? Can we "shrink" it to a small checkbox placed right after client's name or inum fields? May be it doesn't even need a caption of its own, just a checkbox, perhaps with a hint explaining its purpose when mouse cursor is hovered over it, or with some visual effect (green halo around it like around a light bulb etc) when it's set?

Another approach could be that when you unset it all fields on all tabs become grayed-out and you can't edit them. That would clearly telegraph to user that the client is disabled, on itself, I guess.

@willow9886

This comment has been minimized.

Contributor

willow9886 commented Oct 9, 2018

"Logo URI", "Policy URI"

I think these could be used quite frequntly... they are presented to the user in an OAUth consent transaction. They should probably be in the general settings

@syntrydy

This comment has been minimized.

Contributor

syntrydy commented Oct 12, 2018

image

image

image

image

image

syntrydy pushed a commit that referenced this issue Oct 12, 2018

syntrydy added a commit that referenced this issue Oct 12, 2018

Merge pull request #1301 from /issues/1262
Enhance Advanced tab #1262

syntrydy pushed a commit that referenced this issue Oct 12, 2018

@syntrydy syntrydy closed this Oct 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment