Space management page UX improvements #100448

thomheymann · 2021-05-23T22:04:01Z

Resolves #39067
Resolves #27743
Resolves #88368

Summary

This PR improves the UX of the manage spaces page by inlining the avatar customisation fields, adding missing form validation, simplifying the URL identifier input and removing unnecessary clutter.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)

elasticmachine · 2021-05-25T17:22:50Z

Pinging @elastic/kibana-security (Team:Security)

legrego

This looks great! I didn't do any copy review, as we discussed doing an interactive session with @gchaps instead

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space.tsx

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space_avatar.tsx

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space.tsx

x-pack/plugins/spaces/public/management/edit_space/manage_space_page.tsx

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space_avatar.tsx

x-pack/plugins/spaces/public/management/lib/validate_space.test.ts

legrego

Code changes LGTM! Let me know if you'd like me to participate in the copy review.

Once copy review is done, we should update this screenshot for the docs:
https://github.com/elastic/kibana/blob/8fa93bc669547a2fa6206cfdd0682096e1d7df09/docs/spaces/images/edit-space.png

thomheymann · 2021-05-27T09:48:38Z

Once copy review is done, we should update this screenshot for the docs:
https://github.com/elastic/kibana/blob/8fa93bc669547a2fa6206cfdd0682096e1d7df09/docs/spaces/images/edit-space.png

Ah, good catch. I've reviewed copy changes with Gail yesterday and have updated the screenshots.

mdefazio

Just one comment on the usage of KeyPadMenu vs EuiCard. (Docs. Not sure it's a deal breaker, but might help us avoid adding the custom styling.

mdefazio · 2021-05-27T12:36:55Z

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space_avatar.tsx

-            onChange={this.onInitialsChange}
-            disabled={this.props.space.imageUrl && this.props.space.imageUrl !== '' ? true : false}
-          />
+          <EuiKeyPadMenu>


Do we need to use KeyPadMenu here? I think perhaps a selectable card is more typical for this type of usage. And then we can remove the custom styling applied to it?

I'm not sure whether the selectable card would work in this instance since it allows multi selection and feels more like it's own panel rather than a form element.

There's an open issue to add this functionality to the keypad menu so the custom styling here is a stopgap solution:
elastic/eui#4000

The design for this is based on:
https://www.figma.com/proto/dBta1q3JgFe3Cfhw5h76Oq/%5BAwaiting-Dev%5D-Spaces-%26-Roles-Solution-Grouping?node-id=20%3A2087&scaling=min-zoom

I'd like to jump in and make a suggestion here since the aim of this is to make UX improvements. When I look at the "Customize Avatar" section, the most prominent thing I see is the (most likely) one-time selection input of text vs image. I have to scan all the rest of the inputs to finally see the rendered Avatar. This Avatar needs to be the most prominent. I care about the final output... what does this look like?

My suggestions is to:
A. Move the rendered Avatar to the left side of the inputs (first instead of last).
B. While I do think this keypad item style of selection is a nice visual, the prominence is too much for this section. I would just not make them have to select between text and an image and just always show all inputs. Yes, the image will override the text and color, but I think that's a given knowing how avatars work. So that when they clear the uploaded image they just easily revert back to the text/color version and they don't have to re-select "Initials"

Sounds good @thomheymann — clearly i'm missing some discussions here :D

Thanks for the suggestion @cchaos! Makes much more sense!

@MichaelMarcialis How do you want to proceed with this design?

Being able to specify a background colour might be desired when uploading a logo / icon as the avatar image so maybe that should always be selectable.

We can of course change the behaviour to force a white or transparent background but this would be a Kibana wide change that could look strange in certain contexts. Transparent backgrounds would hide the shape of the space avatar which would make it difficult to recognise as such. Using a white background would pretty much do the same in light mode and look quite harsh in dark mode.

I agree -- this is closest to the current behavior as well, so our ux improvements won't end up surprising users

Being able to specify a background colour might be desired when uploading a logo / icon as the avatar image so maybe that should always be selectable.

Good point. Better to not adversely affect existing avatars or remove existing features/capabilities. I'm good with having the color picker in both initials and image modes. Let me know if you need any design support or if you feel comfortable making the changes directly.

@MichaelMarcialis Could you please provide me with a design for these changes?

@thomheymann: Certainly. I've put together a quick updated mockup for the changes discussed above. Let me know if this works for your needs or if you have any questions. Thanks!

Thanks Michael, I've updated the form accordingly.

thomheymann · 2021-05-27T12:59:59Z

@elasticmachine merge upstream

thomheymann · 2021-06-01T15:15:03Z

@elasticmachine merge upstream

MichaelMarcialis

Thanks for making those changes we discussed, @thomheymann! This is looking good. I've left a few comments/questions below for your review:

The page max-width we discussed previously doesn't appear to be in place currently. Would it be possible to add? Should be as easy as adding a restrictWidth={956} prop if you're using the new EUI page component.
When in image mode, if the user adds a file to the file picker, switches back to initial mode and then back again to image mode, the image continues to show in the avatar preview but the file name gets removed from the file picker. Would it be possible to keep the file name intact in the file picker after switching between avatar modes?
It appears that the avatar preview background color does not get applied until initials or an image is provided. Instead, can we always apply the selected background color regardless if initials or an image are provided (and just rely on the question mark to indicate the missing initials/image)?
Are 1) the suggested placements of the delete space button, 2) the conditional appearance of the save changes bottom bar, and 3) the ability to pick a random color to be considered for a future phase/PR? No problem if so; just asking as they were small suggested improvements in the design.

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space.tsx

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space_avatar.tsx

x-pack/plugins/spaces/public/management/edit_space/manage_space_page.tsx

thomheymann · 2021-07-12T14:56:47Z

Thanks for the thorough feedback @MichaelMarcialis. I've made all the changes where possible.

The following two I wasn't able to implement:

When in image mode, if the user adds a file to the file picker, switches back to initial mode and then back again to image mode, the image continues to show in the avatar preview but the file name gets removed from the file picker. Would it be possible to keep the file name intact in the file picker after switching between avatar modes?

Unfortunately, this isn't possible since the filename isn't stored. As soon as the file field is hidden from the page it won't be displayed anymore. We have the same issue when editing

Are 1) the suggested placements of the delete space button, 2) the conditional appearance of the save changes bottom bar, and 3) the ability to pick a random color to be considered for a future phase/PR? No problem if so; just asking as they were small suggested improvements in the design.

Yeh, these are wider changes that justify a separate issue.

Feedback has already been addressed

legrego

LGTM!

MichaelMarcialis

Thanks for making those updates, @thomheymann. Leaving you some additional comments:

With these recent changes, the avatar preview no longer displays a question mark when the user is in image mode with no image has been applied. Would it be possible to restore the question mark in the preview in this circumstance?
I'm not sure if this issue is due to the updates, but I noticed in image mode that interacting with the file picker's clear button removes the file name from the file picker, but it does not clear the image from the avatar preview. Can we hook it up so that clearing the file picker will also clear the image from the avatar preview? Otherwise, users have no means with which to clear an image other than overwriting, which could create some confusion.
Regarding the file picker status not being stored when the user traverses between initial/image modes, is there any way this could be remedied? I worry that it could be confusing from a user standpoint to have a previously populated file name disappear, despite the image from that file still showing in the preview. Additionally, the file picker component needs to be populated in order for the clear button to show, which should be the mechanism with which users can clear an image from the avatar (as described above).

Once the above is addressed, I'm good to approve. Otherwise, let me know if you want me to open an issue for those additional design items discussed in my previous review or if you are already tracking them. Thanks!

MichaelMarcialis · 2021-07-13T20:41:59Z

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space.tsx

+            labelAppend={i18n.translate('xpack.spaces.management.manageSpacePage.optionalLabel', {
+              defaultMessage: 'Optional',
+            })}


Apologies. I forgot to also mention this string should be wrapped in a <EuiText color="subdued" size="xs">. Possible to add?

thomheymann · 2021-07-19T09:35:50Z

Regarding the file picker status not being stored when the user traverses between initial/image modes, is there any way this could be remedied? I worry that it could be confusing from a user standpoint to have a previously populated file name disappear, despite the image from that file still showing in the preview. Additionally, the file picker component needs to be populated in order for the clear button to show, which should be the mechanism with which users can clear an image from the avatar (as described above).

@MichaelMarcialis Agreed that it looks strange having an empty file picker even though an image has been uploaded when editing an existing space. Unfortunately we don't control the value of the file picker since the browser sets it. The only alternative I can think of would be to show the uploaded image with a clear button instead of the file picker but then the design as a whole doesn't really make sense anymore since we'd be showing two avatars.

If it's any consolation, we're not making it worse - This is as-is behaviour and we currently have the same issue.

Let me know how you want to proceed.

kibanamachine · 2021-07-19T11:39:08Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`spaces`	251	240	-11

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`spaces`	276.8KB	265.2KB	-11.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`spaces`	42.4KB	42.3KB	-61.0B

History

💚 Build #137885 succeeded eb998e1
💔 Build #137851 failed bc3df22
💚 Build #136608 succeeded 8ef4b24
💔 Build #136592 failed 53ed64a
💔 Build #136587 failed 95bd4d5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

MichaelMarcialis

@MichaelMarcialis Agreed that it looks strange having an empty file picker even though an image has been uploaded when editing an existing space. Unfortunately we don't control the value of the file picker since the browser sets it.

Understood. I was just thinking that since we're storing the uploaded image in some way (as it continues to show when switching between initial/image modes), we could also somehow store and restore the file name to the file picker when switching back and forth between initial/image modes. But if that's not technically possible, I'm good with changes made here.

Thanks again for all of your work on this!

* Updated spaces management page * Fixed test failures * updated snapshot * Added suggestions form code review * Fixed unit test * Review suggestion elastic#2 * WIP * Fix build errors * fix type * remove test for popup that doesnt exist anymore * fix test * fix a11y issues * fix a11y issue * Removed unused css * Fix functional test * Added suggestions from code review * Fix typescript errors * Added suggestions from code review Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-07-20T08:36:05Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…y-show-migrate-to-authzd-users * 'master' of github.com:elastic/kibana: (187 commits) Space management page UX improvements (elastic#100448) [Reporting] Unskip flaky test when downloading CSV with "no data" (elastic#105252) Update dependency @elastic/charts to v33 (master) (elastic#105633) [Observability RAC] Improve alerts table columns (elastic#105446) Introduce `preboot` lifecycle stage (elastic#103636) [Security Solution] Invalid kql query timeline refresh bug (elastic#105525) skip flaky suite (elastic#106121) [Security Solution][Endpoint] Fix UI inconsistency between isolation forms and remove display of Pending isolation statuses (elastic#106118) docs: APM RUM Source map API (elastic#105332) [CTI] Adds indicator match rule improvements (elastic#97310) [Security Solution] update text for Isolation action submissions (elastic#105956) EP Meta Telemetry Perf (elastic#104396) [Metrics UI] Drop partial buckets from ALL Metrics UI queries (elastic#104784) Remove beta admonitions for Fleet docs (elastic#106010) [Observability RAC] Remove indexing of rule evaluation documents (elastic#104970) Parameterize migration test for kibana version (elastic#105417) [Alerting] Allow rule to execute if the value is 0 and that mets the condition (elastic#105626) [ML] Fix Index data visualizer sometimes shows wrong doc count for saved searches (elastic#106007) [Security Solution] UX fixes for Policy page and Case Host Isolation comment (elastic#106027) [Security Solution]Memory protection configuration card for policies integration. (elastic#101365) ... # Conflicts: # x-pack/plugins/reporting/public/management/report_listing.test.tsx # x-pack/plugins/reporting/public/management/report_listing.tsx

* Updated spaces management page * Fixed test failures * updated snapshot * Added suggestions form code review * Fixed unit test * Review suggestion #2 * WIP * Fix build errors * fix type * remove test for popup that doesnt exist anymore * fix test * fix a11y issues * fix a11y issue * Removed unused css * Fix functional test * Added suggestions from code review * Fix typescript errors * Added suggestions from code review Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Thom Heymann <[email protected]>

Updated spaces management page

0a29cd0

thomheymann force-pushed the manage-spaces branch from 16cd550 to 0a29cd0 Compare May 25, 2021 13:38

thomheymann changed the title ~~[WIP] Updated spaces management page~~ Updated spaces management page May 25, 2021

legrego self-requested a review May 25, 2021 17:12

legrego added Feature:Security/Spaces Platform Security - Spaces feature release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.14.0 v8.0.0 labels May 25, 2021

thomheymann added 2 commits May 25, 2021 20:21

Fixed test failures

8a2e9d6

updated snapshot

5ff57d5

thomheymann marked this pull request as ready for review May 25, 2021 17:22

thomheymann requested review from a team as code owners May 25, 2021 17:22

legrego reviewed May 25, 2021

View reviewed changes

thomheymann changed the title ~~Updated spaces management page~~ Space management page UX improvements May 26, 2021

thomheymann added 2 commits May 26, 2021 12:03

Added suggestions form code review

cc85b6e

Fixed unit test

f51b442

thomheymann requested a review from legrego May 26, 2021 14:49

legrego reviewed May 26, 2021

View reviewed changes

Review suggestion elastic#2

7ef225d

mdefazio previously requested changes May 27, 2021

View reviewed changes

Merge branch 'master' into manage-spaces

e27789a

kibanamachine and others added 3 commits June 1, 2021 11:15

Merge branch 'master' into manage-spaces

8024e7a

Merge branch 'master' of github.com:elastic/kibana into manage-spaces

65d17c2

WIP

208e4e9

thomheymann requested a review from mdefazio July 5, 2021 09:37

thomheymann added 9 commits July 5, 2021 10:52

Fix build errors

ec926a7

fix type

9b2d4c8

remove test for popup that doesnt exist anymore

9cf8361

fix test

b63994f

fix a11y issues

681d9c1

fix a11y issue

95bd4d5

Removed unused css

5c3f3de

Merge branch 'master' of github.com:elastic/kibana into manage-spaces

53ed64a

Fix functional test

8ef4b24

MichaelMarcialis reviewed Jul 9, 2021

View reviewed changes

thomheymann added 2 commits July 12, 2021 10:41

Merge branch 'master' of github.com:elastic/kibana into manage-spaces

7668cdb

Added suggestions from code review

bc3df22

Fix typescript errors

eb998e1

legrego self-requested a review July 13, 2021 15:21

legrego approved these changes Jul 13, 2021

View reviewed changes

MichaelMarcialis reviewed Jul 13, 2021

View reviewed changes

thomheymann added 2 commits July 19, 2021 09:47

Merge branch 'master' of github.com:elastic/kibana into manage-spaces

21168e7

Added suggestions from code review

972a2f9

MichaelMarcialis approved these changes Jul 19, 2021

View reviewed changes

thomheymann merged commit 1f5be1e into elastic:master Jul 20, 2021

kibanamachine mentioned this pull request Jul 20, 2021

[7.x] Space management page UX improvements (#100448) #106211

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Space management page UX improvements #100448

Space management page UX improvements #100448

thomheymann commented May 23, 2021 •

edited

Loading

elasticmachine commented May 25, 2021

legrego left a comment

legrego left a comment

thomheymann commented May 27, 2021

mdefazio left a comment

mdefazio May 27, 2021

thomheymann May 27, 2021

cchaos May 27, 2021

mdefazio May 27, 2021

thomheymann May 27, 2021

legrego Jun 3, 2021

MichaelMarcialis Jun 3, 2021

thomheymann Jun 21, 2021

MichaelMarcialis Jun 23, 2021

thomheymann Jul 5, 2021

thomheymann commented May 27, 2021

thomheymann commented Jun 1, 2021

MichaelMarcialis left a comment

thomheymann commented Jul 12, 2021

legrego left a comment

MichaelMarcialis left a comment

MichaelMarcialis Jul 13, 2021

thomheymann commented Jul 19, 2021

kibanamachine commented Jul 19, 2021

MichaelMarcialis left a comment

kibanamachine commented Jul 20, 2021

Space management page UX improvements #100448

Space management page UX improvements #100448

Conversation

thomheymann commented May 23, 2021 • edited Loading

Summary

Checklist

elasticmachine commented May 25, 2021

legrego left a comment

Choose a reason for hiding this comment

legrego left a comment

Choose a reason for hiding this comment

thomheymann commented May 27, 2021

mdefazio left a 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

thomheymann commented May 27, 2021

thomheymann commented Jun 1, 2021

MichaelMarcialis left a comment

Choose a reason for hiding this comment

thomheymann commented Jul 12, 2021

legrego left a comment

Choose a reason for hiding this comment

MichaelMarcialis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomheymann commented Jul 19, 2021

kibanamachine commented Jul 19, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

MichaelMarcialis left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 20, 2021

💚 Backport successful

thomheymann commented May 23, 2021 •

edited

Loading