UI: Confirm Action as a popover #6741

joshuaogle · 2019-05-15T19:28:09Z

Background

Vault stores a lot of sensitive data, so it is generally good to warn a user when an action that they take might have serious consequences. We do this in the UI by giving the user some context around this action and having them confirm that they are sure.

Prior to Vault v1.2, this design was extremely simple, and appeared when the user clicks on a "Delete" button (or "Revoke", or anything similar) on a list row or anywhere else, such as a "show" page

Along come Toolbars

In Vault v1.2, we are introducing the concept of Toolbars to the UI. The Toolbar gives us a consistent way to present page-level actions, including ones that may require confirmation. Unfortunately, the old design for these confirmations doesn't work well with the limited space, so in the HashiCorp Design System we have created a more consistent and clear way to confirm these actions with users, called "Confirm Popovers".

This PR includes the new design for this component, which also gave us an opportunity to write new copy making the actions even clearer to the user. The default popover and copy looks like this:

Wherever we have clearer context we can provide the user, they would be more customized, like this:

Note: We plan to utilize a "sliding menu" similar to the Namespace Picker for use in dropdown menus, but because the default solution in this PR is still very usable, we will tackle that later.

joshuaogle · 2019-05-15T19:28:57Z

ui/app/styles/components/popup-menu.scss

+    min-width: 0;
+    padding: 0;
+    width: 2.5rem;
+  }


I think it might be good to introduce an "Icon button" style rather than do it this way, but this is an easy reset for now

joshuaogle · 2019-05-15T19:29:30Z

ui/app/templates/components/auth-info.hbs

+          <CopyButton @clipboardText={{auth.currentToken}} class="link" @buttonType="button" @success={{action (set-flash-message 'Token copied!')}}>
+            Copy token
+          </CopyButton>
+        </li>


Gathering the "destructive actions" at the bottom of the list, so moving these up

joshuaogle · 2019-05-15T20:36:27Z

ui/app/templates/components/role-aws-edit.hbs

+          Delete role
+        </ConfirmAction>
+      {{/if}}
+      {{#if (or model.canUpdate model.canDelete)}}


Any idea why the Edit button isn't showing up with just {{#if model.canUpdate}}?

Hmm it should - I think previously it had this because delete was on the edit page. I can take a look.

Ok, it's because the model it's a capabilities object, and the actual attribute is canEdit, so just #if model.canEdit should work here - looks like that's true of other secret models too.

vault/ui/app/models/role-aws.js

Line 67 in d7e441a

canEdit: alias('updatePath.canUpdate'),

meirish · 2019-05-16T18:10:51Z

ui/app/templates/components/pki-cert-popup.hbs

+            @buttonClasses="link is-destroy"
+            @onConfirmAction={{action "delete" item}}
+            @confirmTitle="Revoke this cert?"
+            @confirmMessage="This may affect access to Vault data."


This could be true, but we really don't have a way to tell what they're using the certs for (whether it's vault authentication or something else).

How about something more generic like "Any services using this cert may be affected."?

I'm trying to give them an idea of the consequences, but certs are a tricky one

meirish · 2019-05-16T18:11:31Z

ui/app/templates/components/pki-cert-show.hbs

+      @buttonClasses="button"
+      @onConfirmAction={{action "delete"}}
+      @confirmTitle="Revoke this cert?"
+      @confirmMessage="This may affect access to Vault data."


Same as above, we don't know how they're using the certs.

meirish · 2019-05-16T18:11:53Z

ui/app/templates/components/transit-key-actions.hbs

+    <ConfirmAction
+      @buttonClasses="toolbar-link"
+      @confirmTitle="Rotate this key?"
+      @confirmMessage="After rotation, new plaintext requests will be encrypted with the new version of the key."


I don't think we want to mention encryption here because the key may not support encrypt / decrypt. Maybe "After rotation, all key actions will default to using the newest version of the key." ?

I think this is the only one I copied word for word from the docs (https://www.vaultproject.io/api/secret/transit/index.html#rotate-key), but I do like the change better. I wonder if we should add that detail there then

meirish · 2019-05-16T18:12:38Z

ui/app/templates/partials/role-pki/popup-menu.hbs

-              buttonClasses="link is-destroy"
-              onConfirmAction=(action "delete" item)
-              confirmMessage=(concat "Are you sure you want to delete " item.id "?")
-              showConfirm=(get this (concat "shouldDelete-" (dot-to-dash item.id)))


👏 nice cleanup here!

meirish · 2019-05-16T18:13:26Z

ui/app/templates/partials/role-pki/form.hbs

@@ -26,16 +26,5 @@
        Cancel
      {{/secret-link}}
    </div>
-    {{#if (and (eq mode 'edit') model.canDelete)}}


So no deletes currently happen on the edit page anymore, is that right?

Yep, that's right. If there is a "show" page we should have it as a link there instead. We do still have a couple places that still need to be moved I think.

ui/app/templates/partials/transit-form-edit.hbs

meirish · 2019-05-16T18:27:38Z

ui/tests/acceptance/secrets/backend/ssh/role-test.js

-    assert.equal(currentRouteName(), 'vault.cluster.secrets.backend.edit', 'navs to the edit page');
-
-    await editPage.deleteRole();
+    await showPage.visit({ backend: path, id: 'role' });


I think we should end up on the show page already here (but it probably doesn't hurt to explicitly nav there).

ui/lib/core/addon/components/confirm-action.js

meirish · 2019-05-16T18:34:58Z

ui/lib/core/addon/components/confirm-action.js

 * @property {String} [disabledMessage=Complete the form to complete this action] - The message to display when the button is disabled.
 *
 */

 export default Component.extend({
-  tagName: 'span',


What do you think about doing tagName: '' and then moving all class and wrapping HTML to the template?

I'd have to pass in the data-attributes explicitly like I did for ToolbarLink (until we can use ...attributes), right? Is that worth it?

You should be able to use ...attributes with this one because it's not one of the built-in template helpers (link-to was the problem with the other one iirc). But up to you if you want to change it - when we move to glimmer components that will be the default way things work so more just trying to shift to that where we can.

ui/lib/core/addon/components/confirm-action.js

meirish

🎆 🏆 ✨ - this looks great and I know it was no small amount of work - thanks for tackling it!

joshuaogle added 2 commits May 13, 2019 22:26

Refactor ConfirmAction

68fb008

Better copy for confirm actions

3e87d93

joshuaogle commented May 15, 2019

View reviewed changes

joshuaogle requested a review from a team May 15, 2019 19:32

joshuaogle added 2 commits May 15, 2019 13:32

Merge branch 'master' into ui-confirm-action-popover

678532d

Move remaining 'Delete' buttons to Toolbar

2551502

joshuaogle commented May 15, 2019

View reviewed changes

Migrate ConfirmAction components in lib

c28d76b

joshuaogle force-pushed the ui-confirm-action-popover branch from 4383495 to c28d76b Compare May 16, 2019 18:08

meirish reviewed May 16, 2019

View reviewed changes

ui/lib/core/addon/components/confirm-action.js Outdated Show resolved Hide resolved

meirish reviewed May 16, 2019

View reviewed changes

ui/lib/core/addon/components/confirm-action.js Outdated Show resolved Hide resolved

meirish reviewed May 16, 2019

View reviewed changes

ui/lib/core/addon/components/confirm-action.js Outdated Show resolved Hide resolved

joshuaogle added 4 commits May 16, 2019 13:09

Updates for ConfirmAction copy

8fd5b97

Merge branch 'master' into ui-confirm-action-popover

767d35b

Move ConfirmAction layout to template

e03ce83

Remove container tag for ConfirmAction

9ed84e7

joshuaogle force-pushed the ui-confirm-action-popover branch from ff880d0 to 9ed84e7 Compare May 16, 2019 21:07

Merge branch 'master' into ui-confirm-action-popover

bad833a

meirish approved these changes May 17, 2019

View reviewed changes

joshuaogle merged commit 6603a86 into master May 17, 2019

joshuaogle deleted the ui-confirm-action-popover branch May 17, 2019 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI: Confirm Action as a popover #6741

UI: Confirm Action as a popover #6741

joshuaogle commented May 15, 2019

joshuaogle May 15, 2019

joshuaogle May 15, 2019

joshuaogle May 15, 2019

meirish May 15, 2019

meirish May 16, 2019

meirish May 16, 2019

joshuaogle May 16, 2019

meirish May 16, 2019

meirish May 16, 2019

meirish May 16, 2019

joshuaogle May 16, 2019

meirish May 16, 2019

meirish May 16, 2019

joshuaogle May 16, 2019

meirish May 16, 2019

meirish May 16, 2019

joshuaogle May 16, 2019

meirish May 16, 2019

meirish left a comment

UI: Confirm Action as a popover #6741

UI: Confirm Action as a popover #6741

Conversation

joshuaogle commented May 15, 2019

Background

Along come Toolbars

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meirish left a comment

Choose a reason for hiding this comment