Skip to content
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

data view side panel explore button #135224

Merged
merged 10 commits into from
Jul 13, 2022
Merged

Conversation

ppisljar
Copy link
Member

@ppisljar ppisljar commented Jun 27, 2022

Summary

Adds explore button to create dataview side panel which creates an ad-hoc dataview.
The button is only shown if allowAdHocDataView prop is set to true.
Clicking the button returns a dataview instance to the caller but does not create a saved object.

resolves #132709

image

Checklist

Delete any items that are not applicable to this PR.

@ppisljar ppisljar requested a review from a team as a code owner June 27, 2022 13:51
@ppisljar ppisljar added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.4.0 WIP Work in progress labels Jun 27, 2022
@ppisljar ppisljar force-pushed the adhocdataviews/sidebar branch from a269e39 to 1fe308f Compare June 27, 2022 14:27
@ppisljar ppisljar requested a review from a team as a code owner June 27, 2022 14:27
@ppisljar ppisljar removed the request for review from a team June 27, 2022 14:55
@ppisljar ppisljar force-pushed the adhocdataviews/sidebar branch from 85526a2 to dfda7e2 Compare June 28, 2022 05:43
@flash1293
Copy link
Contributor

I think this is great as a first step 👍

I think it would make sense to show a tooltip when hovering over the button (something like "Use Kibana without persisting the data view. You can always save it later on")

@ppisljar
Copy link
Member Author

currently the tooltip says Explore creates a DataView without persisting it in a saved object. @ghudgins what should we set this to ?

@ppisljar ppisljar force-pushed the adhocdataviews/sidebar branch from 1b6c0c9 to ed32da8 Compare June 28, 2022 10:44
@ppisljar ppisljar removed the WIP Work in progress label Jun 28, 2022
@ppisljar ppisljar requested a review from ghudgins June 28, 2022 12:57
Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how much feedback you want on the UX right now, but clicking "explore" doesn't select the ad-hoc data view (the previously-selected data view stays active), and it still says "Saved ." Can we make the notification text conditional on whether a saved object was actually created, and switch to the ad-hoc data view when "explore" is clicked?

@ppisljar
Copy link
Member Author

@lukasolson no application is integrated with this jet, so explore button should never show up yet. If you would to enable it you would also need to correctly handle the ad hoc dataview (show it in the dropdown etc).

@ghudgins what do you think about changing the notification text ?

Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kertal
Copy link
Member

kertal commented Jul 5, 2022

I'm not sure how much feedback you want on the UX right now, but clicking "explore" doesn't select the ad-hoc data view (the previously-selected data view stays active), and it still says "Saved ." Can we make the notification text conditional on whether a saved object was actually created, and switch to the ad-hoc data view when "explore" is clicked?

+1 for that, I'm currently having hands on this PR, and I think the text should be conditional
Also have a question, the ad-hoc data view is getting an id ... if it is persisted is this id kept?
Another question, the wording "Save data view to Kibana" are we sure users know what we mean with that, just was wondering if we have similar wording in other parts of our software?

@flash1293
Copy link
Contributor

and switch to the ad-hoc data view when "explore" is clicked?

I think this is out of scope for this PR - the consumer has to take care of that (Discover / Lens / ...)

@ppisljar
Copy link
Member Author

ppisljar commented Jul 5, 2022

yes, adhoc data view gets an id and its not going to change if the dataview is persisted.

@kertal
Copy link
Member

kertal commented Jul 5, 2022

FYI, very hacky, but I managed to convince consumer Discover to use it, here is the first ad-hoc one that worked
Bildschirmfoto 2022-07-05 um 20 31 27
:

@KOTungseth KOTungseth added the ui-copy Review of UI copy with docs team is recommended label Jul 5, 2022
@mdefazio
Copy link
Contributor

mdefazio commented Jul 6, 2022

What if the 'Explore' button instead said 'Preview data view'? And then the tooltip text:

Preview this DataView without creating a saved object

Could maybe even go so far as to shorten them to be Preview | Save data view?

If we show a toast following these actions, when clicking 'Preview' we should mention it's not saved yet.

This is a preview of the data view. To use it in Kibana, save it first."

^^ That's not great copy, so just getting the idea down.

@flash1293
Copy link
Contributor

flash1293 commented Jul 6, 2022

We planned to also allow the user to save a Lens vis / discover search together with the ad-hoc data view - it won't show up in the data view listing, it's basically part of the vis/search. Not sure whether "preview" fully captures this, it sounds like you have to turn it into a "real" data view before being able to do anything else with it

@mdefazio
Copy link
Contributor

mdefazio commented Jul 6, 2022

At what point would this ad-hoc view be discarded? If I'm able to create a Lens vis, and save it, does that then also save this data view? Apologies as I think I'm jumping into the middle here.

How can I get this to show up if I run this PR locally? Or can we include the toast(s) in the pr description?

Let me know if I'm expanding outside the scope of this PR :)

@gchaps
Copy link
Contributor

gchaps commented Jul 6, 2022

+1 to Preview | Save data view

Here's another suggestion for the tooltip:

View what's inside this data view before saving it.

I don't usually see a tooltip on a button. Is there another way we can present this information?

@ppisljar
Copy link
Member Author

ppisljar commented Jul 7, 2022

@kertal can you share your code in a PR ? i am wondering why runtime fields etc are not working as it should come out of the box .

@kertal
Copy link
Member

kertal commented Jul 7, 2022

@ppisljar sure! #135770

@mdefazio
Copy link
Contributor

mdefazio commented Jul 7, 2022

I don't usually see a tooltip on a button. Is there another way we can present this information?

Discussed with @gchaps and we are aligned on showing this tooltip on a button. There was some initial confusion on how this was presented, but having an EuiTooltip on a button is fine.

@mdefazio
Copy link
Contributor

mdefazio commented Jul 7, 2022

here you can test it...

This is so awesome 🙌

@flash1293
Copy link
Contributor

@gchaps Can you explain why you think "Preview" is the right wording in the context of #135224 (comment) ? I can totally imagining this way being a common approach of using Discover / Visualize at some point and at least to me "Preview" has a kind of temporary / "not the actual thing" feeling to it

@mdefazio
Copy link
Contributor

mdefazio commented Jul 7, 2022

Here's a flow showing a complete path from creating an ad hoc view to saving it. When you all get a chance, do you mind reviewing it to make sure I am understanding the complete context?

Hopefully this will also help lend some insight into what to label the buttons.

I know that some of this is outside this PR and depends on the consumer of this, but thought it'd be good to nail down our preferred full flow for it regardless.

Again, perhaps outside the scope here, but I didn't see how to save an ad hoc view once I've gone down the path of 'exploring' it.

Whimsical flow
Screenshot:
image

@mattkime
Copy link
Contributor

mattkime commented Jul 7, 2022

This is a nice step forward but I wonder if long term it would make more sense to have a smaller UI for creating ad hoc data views. There's a lot of functionality in the main data view creation flow that we could reasonably expect a user to ignore if they're working quickly - ad hoc data views are unlikely to need a name (could just use the index pattern), field formatting and other field level options.

@flash1293
Copy link
Contributor

@mattkime That's definitely the plan - @cchaos actually prepared some mockups for that already, but we decided to go with just the bare functionality and the existing UI first to de-risk shipping this. Then in a follow-up we can add an inline-ui (it's planned to live in the data view selection popover of unified search)

@ghudgins
Copy link
Contributor

ghudgins commented Jul 7, 2022

I agree with @flash1293 that Preview seems too temporary given you can save the UI with an ad hoc data view permanently. Said another way, I might click this temporary option and be totally happy that it's only in use on this one visualization and never return to save it as a data view

I also agree with @gchaps that Explore is ambiguous when compared to the other option which is pretty clear (Save this data view).

Maybe we could come up with other ideas that are more explicit? I can't seem to get away with not changing the existing "Save data view to Kibana" because, technically, they're both data views in different states of re-use

Here's an alternative:

  • Use this selection | Save selection as data view
  • Use this selection | Save data view to Kibana (what it is today)
  • Use this selection | Save to Data View menu
  • Use without saving | Save to Data View Menu

...but i'm not sure I love it. ideas?

@flash1293
Copy link
Contributor

Maybe we need to introduce the notion of the “data view library” to disambiguate?

@ghudgins
Copy link
Contributor

ghudgins commented Jul 7, 2022

Maybe we need to introduce the notion of the “data view library” to disambiguate?

isn't the data view menu the library in this case? that's where you'll see data views. not sure we need another listing experience....but i'm also not sure the menu has a name for anyone but us

@flash1293
Copy link
Contributor

That’s what I meant - we call the data views management page the “data view library” and refer to it as such everywhere. “Save to library” will have a clear meaning and it’s consistent with visualize library

@gchaps
Copy link
Contributor

gchaps commented Jul 7, 2022

@flash1293 Sorry I missed your previous comment about "preview", and I see your point. Given your and Graham's suggestions, I wonder if this combination might work:

Use without saving | Save to library

@jughosta
Copy link
Contributor

jughosta commented Jul 7, 2022

Hi! Sorry for jumping in here. As I understand we are introducing ad-hoc data views to make it easier/faster to start seeing data, right? I think having an option to not save a data view in a modal with "Create Data View" title looks confusing. Also we are integrating it inside the flyout which can be opened only after pressing "Create a data view" button in the data view switcher. User would have to make an extra decision at the end of the flow.

What do you think about introducing a separate entry point? This way user decides first to save or not to save and then proceeds with entering an index pattern and a time field. And we could render a modal title and a single bottom button depending on that entry point.

Screenshot 2022-07-07 at 19 44 43

@ghudgins
Copy link
Contributor

ghudgins commented Jul 7, 2022

@jughosta we have plans for this in a subsequent phase. happy to share some designs to get your feedback! edit: this first phase is really about getting the minimum user interface in so we can work through the backend changes

@jughosta
Copy link
Contributor

jughosta commented Jul 7, 2022

@jughosta we have plans for this in a subsequent phase. happy to share some designs to get your feedback!

Great! Thanks @ghudgins !

@ppisljar
Copy link
Member Author

This is a nice step forward but I wonder if long term it would make more sense to have a smaller UI for creating ad hoc data views. There's a lot of functionality in the main data view creation flow that we could reasonably expect a user to ignore if they're working quickly - ad hoc data views are unlikely to need a name (could just use the index pattern), field formatting and other field level options.

i would argue that everything you want to do with the saved one you want to do with the ad-hoc one. Yes possibly we want to have a quicker way to get there, but making some options not available at all just sets us back and gets us in this bad place again where the user experience is different based on wether you work with ad hoc or normal dataview.

thats also the reason why i think the flow diagram @mdefazio posted is over complicated. It brings in lens, discover, ....
I think its important here to understand that what happens with the dataview once its created is completely open and should not depend on wether the dataview is saved or ad hoc. For user it shouldn't matter that he is creating a lens chart of the saved search that uses an ad hoc dataview for example.

@ppisljar
Copy link
Member Author

@kertal it looks fieldAttrs can't be extended on the dataview instance returned by the data view editor. cc @mattkime do you have any idea why that would be ? if i override fieldAttrs inside the create method with an empty object {} this starts to work. I don't understand how this worked in my PoC where i tested this :|

@ppisljar
Copy link
Member Author

ppisljar commented Jul 12, 2022

@ghudgins to move forward with this, what do you think about:

  • we rename the button to use without saving
  • the tooltip says Use this DataView without creating a Kibana saved object

we leave the save dataview button for now and in a follow up we can rename the dataview management to dataview library and update all the other texts accordingly.

@ghudgins
Copy link
Contributor

sounds reasonable. I agree the library change is a bigger one

@gchaps
Copy link
Contributor

gchaps commented Jul 12, 2022

We write data view as two words and try to avoid using Kibana in our copy. So if it is not necessary here, I suggest:

Use this data view without creating a saved object.

@ppisljar ppisljar removed the request for review from ghudgins July 13, 2022 03:14
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
dataViewEditor 30.7KB 31.6KB +928.0B

Page load bundle

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

id before after diff
dataViewEditor 10.2KB 10.2KB +87.0B
Unknown metric groups

API count

id before after diff
dataViewEditor 14 15 +1

History

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

@ppisljar ppisljar merged commit 6f9d771 into elastic:main Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes ui-copy Review of UI copy with docs team is recommended v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dataviews] allow opening create data view sidebar without persisting the data view