View/edit modes for dashboard #10585

stacey-gammon · 2017-02-25T17:44:44Z

What
Create two separate modes in dashboard, "View" and "Edit", so we can optimize the user experience around each.

Why

We believe the default, majority use case is consumption. Most of the time users want to view, not edit, their dashboards.
There also exist use cases where the dashboard creator and viewer are separate individuals. By defaulting into a View only mode, we can prevent showing unnecessary controls to viewers, reducing the likelyhood they will accidentally break or change something.
Improved UX/UI for the 'view only' use case:
- No distracting border showing on mouse-over if the user just wants to drill in to a specific time on a dashboard
- Prevent accidentally deleting panels from stray clicks.
- Hide irrelevant controls and navigations menu items, making the dashboard easier to understand at a glance.
Using limited navigation menu items in each mode allows us to add more menu items without that top nav bar getting too busy or crowded.
A poor man's implementation of Supporting "locking" of dashboards, making them read-only #2683. Will not be sufficient for everyone, and we'll have to make it clear we are not offering true security, but may offer a halfway decent workaround for some use cases (as noted by some in that issue).

Future benefits

Paves the way for an object level security workflow. In that situation, users without appropriate permissions would be able to view, but would not have an edit button available, and hence be in view mode only.
Paves the way for a potential auto save feature. An explicit edit mode would greatly decrease the chance the wrong user would accidentally update a dashboard, if their intention was to view only.

What this is not
This feature and discussion is separate from optimizing the user experience around embedded or full screen mode. That feature will require further thought, but at a high level, we'll likely expose options during the share and link creation process so embedded links can be customized (e.g. show search bar, show timepicker, allow resize and move).

fixes #9431

cjcenizal · 2017-02-26T20:42:03Z

@stacey-gammon I think this works really, really well. Let me explain why.

Why this works

Here's how web apps typically present this flow to users when they're editing generic text-based documents, e.g. a blog post (see examples in #9431 (comment)):

The user is usually given Save and Cancel buttons, which is what you've provided in this PR. To be explicit to the point of redundancy, here's how this UX maps to Dashboard:

This is the same as what you've built but with the order of the buttons changed around. So I think this PR has established the right UX, for three reasons:

It feels right to me.
It follows a document-editing pattern that's been established by other web apps.
We can apply it to other view/editing flows beyond just Dashboards.

Save and Cancel UX

The Save UX looks good to me but I think we can improve the Cancel UX.

When a user clicks Save, the user has the opportunity to change some metadata including the Dashboard name, confirms the save, and then gets sent back to the View mode, with a confirmation that the save action was successful:

I think the way we currently handle Cancel is a little confusing, because we're actually prompting the user to save:

Instead, I think we should simply confirm that canceling is what the user wants to do. This is a little clearer and is more in-line with what the user would expect:

stacey-gammon · 2017-02-27T20:50:40Z

@cjcenizal Do we still want to call out changes made to filter/query/time picker? If so, in that case should it say:

Are you sure you want to cancel and lose your changes, including changes made to your [filter, query, and/or timepicker]

stacey-gammon · 2017-02-27T20:52:30Z

@cjcenizal One more design question - Do we still want No, keep working to be the default option, as it's "safer"? I feel like it looks weird like this. We already have the safety check, I think it's okay to make the intended action the default.

cjcenizal · 2017-02-28T01:16:27Z

Yes, I think we should still let the user know that these parts of the visualization have changed. Let's use the message: "Are you sure you want to cancel and lose your changes? This includes changes made to your [filter, query, and/or timepicker]."

I think we should put "Yes, lose changes" on the right side and make that a Primary Button. We should assume users are making deliberate decisions and our UI should support the fulfillment of these intentions (while still giving them the opportunity to recognize and correct mistakes). Since this is the confirmation of the user's intent, we should make it the most prominent choice.

But because this is a potentially destructive action (losing valuable work) we shouldn't make this a completely frictionless experience. We should avoiding autofocusing these buttons, or if one is auto-focused, it should be the "No, keep working" button.

We could also try making the "Yes, lose changes" button into a Danger Button, which will stand out to color-sighted users and let them know they should pay attention here. I wonder if this is too much though?

alexfrancoeur · 2017-02-28T16:27:13Z

@stacey-gammon I know we discussed adding clone back in as a separate PR. Did you end up creating one? I don't want to lose track of this. I believe it makes the re-using of a dashboard significantly more intuitive. It is also something we might want to explore for visualizations.

stacey-gammon · 2017-02-28T16:40:57Z

I agree @alexfrancoeur. I haven't gotten around to it yet and probably won't before Elasticon. Filed an issue here to track: #10621.

stacey-gammon · 2017-02-28T17:37:22Z

Hmmm, console test error in selenium. Can't see how this PR would have affected that:

app_should show the default request.png"
>> FAIL: chrome on any platform - kibana - console app - console app - should show the default request (42473ms)
Error: tryForTime timeout: NoSuchElement: An element could not be located on the page using the given search parameters.
    at /var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/leadfoot/lib/findDisplayed.js:37:21
    at /var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/dojo/_debug/Promise.ts:393:16
    at run (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/dojo/_debug/Promise.ts:237:8)
    at /var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/dojo/_debug/nextTick.ts:44:4
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)
    at Command.findDisplayed (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/leadfoot/Command.js:23:10)
    at Command.prototype.(anonymous function) [as findDisplayedByCssSelector] (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/leadfoot/lib/strategies.js:28:16)

jenkins test this

stacey-gammon · 2017-02-28T18:26:20Z

Looks like another unrelated error:

Running "esvm:test" (esvm) task
starting up "test" cluster
INFO -  - cluster - Downloading & installing from "master" branch.
Fatal error: unexpected end of file�
runbld>>> <<<<<<<<<<<< SCRIPT EXECUTION END <<<<<<<<<<<<
runbld>>> DURATION: 383119ms
runbld>>> STDOUT: 120861 bytes
runbld>>> STDERR: 6641 bytes
runbld>>> WRAPPED PROCESS: FAILURE (3)

jenkins test this

stacey-gammon · 2017-03-01T17:30:56Z

jenkins test this

stacey-gammon · 2017-03-02T19:06:34Z

'''

18:58:24.679: Setting absolute range to 2015-09-19 06:31:44.000 to 2015-09-23 18:31:44.000
18:58:26.739: time picker open: false
18:58:26.739: --Opening time picker
18:58:26.740: in findTestSubject: [data-test-subj~="globalTimepickerButton"]
18:58:26.859: --Clicking Absolute button
18:58:26.927: Taking screenshot "/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/test/screenshots/failure/failure_1488481106927_dashboard without stored timed_Does not set the time picker on open.png"
>> FAIL: chrome on any platform - kibana - dashboard app - dashboard time - dashboard without stored timed - Does not set the time picker on open (2373ms)
UnknownError: [POST http://localhost:4444/wd/hub/session/22c1f4ec6a42ed21e6c8d7ae74a4b3e5/element/0.5340270116163361-13/click] unknown error: Element is not clickable at point (260, 298). Other element would receive the click: <div class="row">...</div>
  (Session info: chrome=56.0.2924.87)
  (Driver info: chromedriver=2.24.417424 (c5c5ea873213ee72e3d0929b47482681555340c3),platform=Linux 3.10.0-514.2.2.el7.x86_64 x86_64)
  at runRequest  <node_modules/leadfoot/Session.js:88:40>
  at <node_modules/leadfoot/Session.js:109:39>
  at new Promise  <node_modules/dojo/_debug/Promise.ts:411:4>
  at ProxiedSession._post  <node_modules/leadfoot/Session.js:63:10>

jenkins test this. Think this might be a transitory issue. Will have to keep a close eye.

stacey-gammon · 2017-03-03T18:35:51Z

Another possible flaky test to keep an eye on:

15:36:08.757: Clicking edit
15:36:08.757: in findTestSubject: [data-test-subj~="dashboardEditMode"]
15:36:09.081: Taking screenshot "/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/test/screenshots/failure/failure_1488555369081_dashboard app_retains dark theme in state.png"
>> FAIL: chrome on any platform - kibana - dashboard app - retains dark theme in state (491ms)
UnknownError: [POST http://localhost:4444/wd/hub/session/dbe2c85442375228f777293bcc8d9057/element/0.3511779830984887-56/click] unknown error: Element is not clickable at point (581, 56). Other element would receive the click: <input parse-query="" input-focus="" kbn-typeahead-input="" ng-model="model.query" placeholder="Filter..." aria-label="Filter input" data-test-subj="dashboardQuery" type="text" class="kuiLocalSearchInput ng-pristine ng-valid ng-isolate-scope ng-touched" ng-class="{'kuiLocalSearchInput-isInvalid': queryInput.$invalid}" autocomplete="off">
  (Session info: chrome=56.0.2924.87)
  (Driver info: chromedriver=2.24.417424 (c5c5ea873213ee72e3d0929b47482681555340c3),platform=Linux 4.4.0-45-generic x86_64)
  at runRequest  <node_modules/leadfoot/Session.js:88:40>
  at <node_modules/leadfoot/Session.js:109:39>
  at new Promise  <node_modules/dojo/_debug/Promise.ts:411:4>
  at ProxiedSession._post  <node_modules/leadfoot/Session.js:63:10>
  at Element._post  <node_modules/leadfoot/Element.js:23:31>
  at Element.click  <node_modules/leadfoot/Element.js:163:15>

alexfrancoeur · 2017-03-15T16:54:28Z

@stacey-gammon We've discussed a few times, but overall the feedback for this iteration of view/edit mode has been positive. I'll be sharing the usability feedback early next week. New users had no problem understanding the concept of view edit. Experienced users sometimes did not immediately find the edit button and were looking in the legend, clicking into the visualization and then clicking edit or simply leaving the page to edit a visualization. Once the concept was understood it was mostly praised and had no problem with the workflow.

I did run into a bug when attempting to check out yesterday. It looks like there is an issue merging the modal dialog boxes and the most recent build of master.

From git://github.com/Stacey-Gammon/kibana
 * branch            view-edit-5 -> FETCH_HEAD
Auto-merging src/ui/public/modals/confirm_modal.js
CONFLICT (content): Merge conflict in src/ui/public/modals/confirm_modal.js
Auto-merging src/core_plugins/kibana/public/dashboard/dashboard.js
Automatic merge failed; fix conflicts and then commit the result.

stacey-gammon · 2017-03-20T13:22:45Z

@alexfrancoeur Conflicts are all merged now. I didn't run into that bug, but added some tests that should catch it if it ever does appear.

and add tests to catch it.

and add tests.

- Merge function only used in one spot inline. Add comments

Instead, export each function individually.

…nuing.

stacey-gammon · 2017-03-23T14:08:05Z

rebased with master and all feedback should be addressed! Any more feedback @kobelb?

kobelb · 2017-03-23T14:12:42Z

@stacey-gammon my only feedback at this time is.... SHIP IT!!! LGTM

Backports PR #10585 **Commit 1:** Start at view/edit mode 4 * Original sha: a89ce34 * Authored by Stacey Gammon <[email protected]> on 2017-02-25T17:24:02Z **Commit 2:** Revert back to save panel - Get rid of `clone` - Get rid of rename * Original sha: cfb3372 * Authored by Stacey Gammon <[email protected]> on 2017-02-25T17:35:05Z **Commit 3:** Move order of top nav component around so no chance of hitting Edit -> Cancel * Original sha: 000e857 * Authored by Stacey Gammon <[email protected]> on 2017-02-25T17:43:07Z **Commit 4:** Update "lose changes" confirmation dialog - Change the string and button text - Set default focus on the cancel button. * Original sha: 542e7bf * Authored by Stacey Gammon <[email protected]> on 2017-02-28T16:03:39Z **Commit 5:** Remove DashboardViewMode It was previously used when viewMode wasn’t stored in appState, in order to keep an unsaved dashboard, that was just saved, in Edit mode. This is unnecessary in the latest iteration. * Original sha: 39ba9e3 * Authored by Stacey Gammon <[email protected]> on 2017-03-01T21:51:09Z **Commit 6:** Pull time filter out of dashboard_state * Original sha: 7875547 * Authored by Stacey Gammon <[email protected]> on 2017-03-02T16:01:16Z **Commit 7:** Fix edit controls bug, add more tests, make other tests more stable. * Original sha: 774267c * Authored by Stacey Gammon <[email protected]> on 2017-03-03T14:26:33Z **Commit 8:** Remove unnecessary logic, unapplied query bug fix, code clean up * Original sha: 9cf402a * Authored by Stacey Gammon <[email protected]> on 2017-03-03T18:19:15Z **Commit 9:** Fix bug on filter reloading, add tests & clean up * Original sha: 97c3054 * Authored by Stacey Gammon <[email protected]> on 2017-03-03T20:30:11Z **Commit 10:** Fix issue with move and remove icons showing up in expanded panel mode and add tests to catch it. * Original sha: b8195c4 * Authored by Stacey Gammon <[email protected]> on 2017-03-17T17:30:06Z **Commit 11:** Fix issue with lose changes not resetting panel state and add tests. * Original sha: f92d8f6 * Authored by Stacey Gammon <[email protected]> on 2017-03-17T20:15:12Z **Commit 12:** General Clean Up - Merge function only used in one spot inline. Add comments * Original sha: badf336 * Authored by Stacey Gammon <[email protected]> on 2017-03-17T20:50:55Z **Commit 13:** address code review comments * Original sha: 0c61fc7 * Authored by Stacey Gammon <[email protected]> on 2017-03-21T14:11:41Z **Commit 14:** Stop using static + class to namespace functions Instead, export each function individually. * Original sha: b0745aa * Authored by Stacey Gammon <[email protected]> on 2017-03-22T13:18:27Z **Commit 15:** Show dashed border on maximized panels in edit mode. * Original sha: ec5393b * Authored by Stacey Gammon <[email protected]> on 2017-03-22T14:09:01Z **Commit 16:** Be more methodical about ensuring the modal dialog hides before continuing. * Original sha: 07a031f * Authored by Stacey Gammon <[email protected]> on 2017-03-22T14:26:56Z **Commit 17:** abide by new no unused vars rule * Original sha: 5e76980 * Authored by Stacey Gammon <[email protected]> on 2017-03-22T16:32:30Z

epixa · 2017-03-23T20:51:10Z

Nice!

Can you update the PR description/title with more details about this? Copying and pasting from previous PRs is probably sufficient so long as the info is up to date.

xycloud · 2017-03-24T03:48:05Z

which kibana version will this feature be merged?

SnchitGrover · 2017-03-24T09:13:00Z

Amazing work everyone. Testing the new build out.
Do you think we can have an xpack security role to which we can disable the edit option? That might solve the issue #1610

epixa · 2017-03-24T13:00:16Z

@xycloud We label the PRs with version numbers when they get merged to indicate target versions.

@SnchitGrover There are some difficulties we still need to overcome with Kibana's architecture before we start bubbling up security into the UI like you describe, but that definitely seems like a good use case!

* Start at view/edit mode 4 * Revert back to save panel - Get rid of `clone` - Get rid of rename * Move order of top nav component around so no chance of hitting Edit -> Cancel * Update "lose changes" confirmation dialog - Change the string and button text - Set default focus on the cancel button. * Remove DashboardViewMode It was previously used when viewMode wasn’t stored in appState, in order to keep an unsaved dashboard, that was just saved, in Edit mode. This is unnecessary in the latest iteration. * Pull time filter out of dashboard_state * Fix edit controls bug, add more tests, make other tests more stable. * Remove unnecessary logic, unapplied query bug fix, code clean up * Fix bug on filter reloading, add tests & clean up * Fix issue with move and remove icons showing up in expanded panel mode and add tests to catch it. * Fix issue with lose changes not resetting panel state and add tests. * General Clean Up - Merge function only used in one spot inline. Add comments * address code review comments * Stop using static + class to namespace functions Instead, export each function individually. * Show dashed border on maximized panels in edit mode. * Be more methodical about ensuring the modal dialog hides before continuing. * abide by new no unused vars rule

stacey-gammon · 2017-05-01T12:53:48Z

In 5.4 we've introduced view and edit modes for your Dashboards. Now when you first open a dashboard, you'll be in view mode. In order to make edits and save them, you'll have to enter edit mode by clicking the edit button in the top navigation. Having these modes allows us to optimize the experience for users whose job is primarily to view the dashboards. No more distracting borders and edit controls on mouse over, or the ability to make an accidental change. You'll also be able to discard edits by clicking cancel. Saving a dashboard will automatically bring you back into view mode.

stacey-gammon mentioned this pull request Feb 25, 2017

Introduce view and edit modes for Dashboards #9431

Closed

stacey-gammon force-pushed the view-edit-5 branch from a18470d to a0ae46f Compare February 28, 2017 16:12

stacey-gammon mentioned this pull request Feb 28, 2017

#10099 adds keyboard handling to close confirm_modal on ESC #10455

Merged

stacey-gammon force-pushed the view-edit-5 branch from ef314e4 to 7012de0 Compare March 1, 2017 21:43

tbragin added the :Sharing label Mar 2, 2017

stacey-gammon force-pushed the view-edit-5 branch 4 times, most recently from 51e4ef8 to 93fdd03 Compare March 2, 2017 18:27

stacey-gammon force-pushed the view-edit-5 branch 4 times, most recently from eb4c3a1 to c430761 Compare March 20, 2017 13:21

stacey-gammon requested review from kobelb and cjcenizal March 20, 2017 13:21

stacey-gammon added the v5.4.0 label Mar 20, 2017

stacey-gammon force-pushed the view-edit-5 branch from c430761 to 07e0029 Compare March 20, 2017 14:06

stacey-gammon added 8 commits March 23, 2017 09:00

Fix issue with move and remove icons showing up in expanded panel mode

b8195c4

and add tests to catch it.

Fix issue with lose changes not resetting panel state

f92d8f6

and add tests.

General Clean Up

badf336

- Merge function only used in one spot inline. Add comments

address code review comments

0c61fc7

Stop using static + class to namespace functions

b0745aa

Instead, export each function individually.

Show dashed border on maximized panels in edit mode.

ec5393b

Be more methodical about ensuring the modal dialog hides before conti…

07a031f

…nuing.

abide by new no unused vars rule

5e76980

stacey-gammon force-pushed the view-edit-5 branch from 9e36fe7 to 5e76980 Compare March 23, 2017 13:00

kobelb approved these changes Mar 23, 2017

View reviewed changes

stacey-gammon merged commit 1cd1dc9 into elastic:master Mar 23, 2017

elastic-jasper mentioned this pull request Mar 23, 2017

[5.x] View edit 5 #10873

Closed

stacey-gammon mentioned this pull request Mar 23, 2017

[5.x] View/edit modes for dashboard #10875

Merged

epixa changed the title ~~View edit 5~~ View/edit modes for dashboard Mar 24, 2017

stacey-gammon mentioned this pull request Mar 28, 2017

Dashboard scrolling on touch-enabled devices often does the wrong thing #10733

Closed

alexfrancoeur mentioned this pull request Mar 30, 2017

[DOCS] Updated documentation for view/edit mode #10957

Closed

stacey-gammon deleted the view-edit-5 branch April 6, 2017 11:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

View/edit modes for dashboard #10585

View/edit modes for dashboard #10585

stacey-gammon commented Feb 25, 2017 •

edited

Loading

cjcenizal commented Feb 26, 2017 •

edited

Loading

stacey-gammon commented Feb 27, 2017

stacey-gammon commented Feb 27, 2017 •

edited

Loading

cjcenizal commented Feb 28, 2017 •

edited

Loading

alexfrancoeur commented Feb 28, 2017

stacey-gammon commented Feb 28, 2017 •

edited

Loading

stacey-gammon commented Feb 28, 2017

stacey-gammon commented Feb 28, 2017

stacey-gammon commented Mar 1, 2017

stacey-gammon commented Mar 2, 2017

stacey-gammon commented Mar 3, 2017

alexfrancoeur commented Mar 15, 2017 •

edited

Loading

stacey-gammon commented Mar 20, 2017

stacey-gammon commented Mar 23, 2017

kobelb commented Mar 23, 2017

epixa commented Mar 23, 2017

xycloud commented Mar 24, 2017

SnchitGrover commented Mar 24, 2017

epixa commented Mar 24, 2017

stacey-gammon commented May 1, 2017

View/edit modes for dashboard #10585

View/edit modes for dashboard #10585

Conversation

stacey-gammon commented Feb 25, 2017 • edited Loading

cjcenizal commented Feb 26, 2017 • edited Loading

Why this works

Save and Cancel UX

stacey-gammon commented Feb 27, 2017

stacey-gammon commented Feb 27, 2017 • edited Loading

cjcenizal commented Feb 28, 2017 • edited Loading

alexfrancoeur commented Feb 28, 2017

stacey-gammon commented Feb 28, 2017 • edited Loading

stacey-gammon commented Feb 28, 2017

stacey-gammon commented Feb 28, 2017

stacey-gammon commented Mar 1, 2017

stacey-gammon commented Mar 2, 2017

stacey-gammon commented Mar 3, 2017

alexfrancoeur commented Mar 15, 2017 • edited Loading

stacey-gammon commented Mar 20, 2017

stacey-gammon commented Mar 23, 2017

kobelb commented Mar 23, 2017

epixa commented Mar 23, 2017

xycloud commented Mar 24, 2017

SnchitGrover commented Mar 24, 2017

epixa commented Mar 24, 2017

stacey-gammon commented May 1, 2017

stacey-gammon commented Feb 25, 2017 •

edited

Loading

cjcenizal commented Feb 26, 2017 •

edited

Loading

stacey-gammon commented Feb 27, 2017 •

edited

Loading

cjcenizal commented Feb 28, 2017 •

edited

Loading

stacey-gammon commented Feb 28, 2017 •

edited

Loading

alexfrancoeur commented Mar 15, 2017 •

edited

Loading