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.

Sign up for GitHub

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

Jump to bottom

[Log Explorer] Add "all" entry to dataset selector #160146

Closed
weltenwort opened this issue Jun 21, 2023 · 11 comments · Fixed by #160971
Closed

[Log Explorer] Add "all" entry to dataset selector #160146

weltenwort opened this issue Jun 21, 2023 · 11 comments · Fixed by #160971
Assignees
@tonyghiani
Labels
Feature:LogsExplorer Logs Explorer feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@weltenwort
Copy link
Member

weltenwort commented Jun 21, 2023 •
edited
Loading

📓 Summary

Once the data stream selector is implemented in #159907, we want to accommodate the use-cases that require looking all log entries simultaneously. To that end we want to add a means to create a broader ad-hoc data view that targets all log data streams. This is an intermediate step on the way to arbitrary multi-selection.

✔️ Acceptance criteria

  • The dataset selector is in one of two modes: "all" or "single".
  • There always is an entry at the top of the integrations list that causes the dataset picker to switch to mode "all".
  • If the user selected "all" on the topmost navigation level, ...
    • the dataset selector synthesizes a data view targeting logs-*-*.
    • the "all" entry is shown as active.
    • the dataset selector is closed.
  • If the user selects any other dataset, the dataset selector is in mode "single", in which a dataview for a specific data stream is created, as before.
  • When the user navigates to Discover without a history state and without a pre-selected data view in the URL state, the dataset selector starts out in mode "all".

🎨 Mock-ups

image

💡 Implementation hints

  • The previous mode is called "single" here in anticipation for the multi-select feature introducing a "multi" mode.
@weltenwort weltenwort added Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Jun 21, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@isaclfreire
Copy link

LGTM!

Unrelated question: 'my application' will refer to the basic integration created by the user?

@weltenwort
Copy link
Member Author

Unrelated question: 'my application' will refer to the basic integration created by the user?

Yes, it might. But it's just a placeholder in here without special significance.

@tonyghiani
Copy link
Contributor

A couple of more specific questions about the design:

  • How do we want to mark the All entry (and probably also the other entries for single/multi selection) as active? There is a specific icon+position we would prefer?
  • I see the All entry has a star icon, do we want to use the star, something different, or no icon at all?

Another question related to the modes single and all, does the user have a different perception between them? As far as I can see for the current state, implementing this option as described would be a similar user journey, with the difference that the loaded data view is from a hardcoded dataset (logs-*-*).
There is any difference on how the user will be able to interact with the selector when the All entry is selected?

@weltenwort
Copy link
Member Author

How do we want to mark the All entry (and probably also the other entries for single/multi selection) as active?

Not sure, but I would guess a context menu entry can have an "active" state? Maybe that would work for us here.

I see the All entry has a star icon, do we want to use the star, something different, or no icon at all?

I just randomly picked a shape from the gallery when drawing this. But I do like the symmetry of having an icon for every entry. Maybe we can find something appropriate in the EUI icon collection?

There is any difference on how the user will be able to interact with the selector when the All entry is selected?

I think for the user the states mainly differ in what the button shows and which entry is shown as active.

Good questions, thank you. Maybe @formgeist or @isaclfreire can give us some tips about the visual aspects.

@formgeist
Copy link
Contributor

I did some exploration on what to show for the All integrations and all datasets selection and perhaps this can work

In context;

CleanShot.2023-06-28.at.14.45.29.mp4

@tonyghiani
Copy link
Contributor

I did some exploration on what to show for the All integrations and all datasets selection and perhaps this can work

Love the icon ❤️
About the title, does "All integrations" clearly communicate what they are selecting? With this selection, they'll see logs from logs-*-* dataset, which includes also the logs from uncategorized dataset, so maybe it could be something like "All logs datasets" or "All logs"?

About the title we show for the nested "All datasets" for integrations, we removed the prefix title [<integration-name>] because it was redundant with the icon, should we still keep it in this particular case?

@formgeist
Copy link
Contributor

About the title, does "All integrations" clearly communicate what they are selecting? With this selection, they'll see logs from logs-- dataset, which includes also the logs from uncategorized dataset, so maybe it could be something like "All logs datasets" or "All logs"?

In this case you're right, it might be better to use "All logs datasets" instead.

About the title we show for the nested "All datasets" for integrations, we removed the prefix title [] because it was redundant with the icon, should we still keep it in this particular case?

Ah, that's right 🤦 I think we can show "All [] datasets" to really emphasize the selection. I'm still considering if the removal of the integration name was wise or not - something I'd like to test this out bit more with others to see what they think. It might not be all that easy to see the integration from the 32px icon itself.

@formgeist
Copy link
Contributor

Then the updated design would look like this;

image

image

@tonyghiani
Copy link
Contributor

LGTM, thanks for updating that so quickly 🙏

@weltenwort weltenwort mentioned this issue Jun 28, 2023
Closed
@tonyghiani
Copy link
Contributor

@formgeist a question regarding the requirement

the "all" entry is shown as active.

How could we mark an entry as active in a way such that it looks the same independently of the type of selection (all, single, multi)?

In particular, how would we mark the "All log datasets" as active, since it already has an icon on the left?

@tonyghiani tonyghiani mentioned this issue Jun 30, 2023
Merged
tonyghiani added a commit that referenced this issue Jul 4, 2023
@tonyghiani @kibanamachine
[Logs+] Add All entry for DatasetSelector (#160971)
b9e2fdb 
## 📓 Summary

Closes #160146 

This PR adds the entry to allow users to select a dataset that creates a
`logs-*-*` dataview.

Although the presentational and UX changes are minimal for the user,
this work lays the foundation for [restoring the dataset selection from
the URL](#160425) and for the
[dataset multi-selection
feature](https://github.com/elastic/observability-dev/issues/2744).

The core changes for this implementation consist of:
- Update DatasetSelector state machine to manage two parallel states: a
`popover` one to manage the navigation on the selector and a `selection`
state that handles the selection modes (currently only `all` and
`single`, but ready to also implement `multi`)
<img width="1522" alt="state-machine"
src="https://github.com/elastic/kibana/assets/34506779/c240e5d5-6a38-4d08-b893-117132477896">

- DatasetSelector is now a controlled component regarding the selection
value: it will react to the injected `datasetSelection` property, and
notify the parent component of any change with the `onSelectionChange`
event handler.
This will allow us to always reinitialize the DatasetSelector state
machine from the URL state and fall back to the chosen selection in case
there is no one to restore.


https://github.com/elastic/kibana/assets/34506779/4887b1d4-63ba-476b-a74f-5b4a9504f939

## Architectural choices

- The state machine will handle the two states in parallel such that we
can switch to available selection modes depending on the interactions
without clashing with the independent selector navigation.
- The `DatasetSelection` data structure is now what represents the state
selection in different modes.
The three available modes (counting also `multi`, but not implemented
yet), differs in mostly any aspect:
  - Internal data structure shape
  - DataViewSpecs composition
  - Payload to encode into a URL state
- Extraction of the presentational values (title, icons, and with
`multi` also the datasets which are currently selected)

With all these differences but the same final purposes of creating an
ad-hoc DataView and storing encoding a URL state to store, applying the
concepts for the Strategy pattern seemed the most sensed option to me.
This allows us to scale and add new selection modes (`multi`) respecting
the existing strategies contract and encapsulating all the concerned
transformations and parsing.

Encoding and decoding of the Dataview id that will be used for restoring
from the URL are already created and will be handy for the upcoming
tasks.

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
@gbamparop gbamparop added Feature:LogsExplorer Logs Explorer feature and removed Feature:Logs UI Logs UI feature labels Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tonyghiani tonyghiani

Labels
Feature:LogsExplorer Logs Explorer feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Logs+] Add All entry for DatasetSelector tonyghiani/kibana
6 participants
@weltenwort @formgeist @gbamparop @elasticmachine @tonyghiani @isaclfreire