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

[Infrastructure UI] Asset Details: Add pins to the metadata table #155190

Closed
jennypavlova opened this issue Apr 18, 2023 · 7 comments Β· Fixed by #161074
Closed

[Infrastructure UI] Asset Details: Add pins to the metadata table #155190

jennypavlova opened this issue Apr 18, 2023 · 7 comments Β· Fixed by #161074
Assignees
Labels
beta Required for a feature to move to beta Feature:Metrics UI Metrics UI feature Feature:ObsHosts Hosts feature within Observability Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@jennypavlova
Copy link
Member

πŸ““ Summary

Inside the single hosts flyout metadata table add an option for the user to pin a table row.

image

🎨 Design

Figma

πŸ‘¨β€πŸ’» Implementation hints

We can check a Pinnable list group or search for an eui table with pins support. The pinned items should be persisted for the current user (locale storage)

βœ”οΈ Acceptance criteria

  • A new column with a pin should be added to the metadata table (first position)
  • A pin icon should appear on row hover
  • The pinned metadata rows should go on top of the table
  • The pinned metadata rows should be persisted on the browser level and not shared with the URL
@jennypavlova jennypavlova added Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services Feature:ObsHosts Hosts feature within Observability labels Apr 18, 2023
@elasticmachine
Copy link
Contributor

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

@kkurstak
Copy link

Regarding the pins I will add a separate issue for it, just have a couple of questions there:
Do we want to persist the pinned table data in the order they are pinned ( from top to bottom) - by moving the row?
Or do we want to keep the table row order as it is and just want to show the blue pin?
Do we have euiTable with pins design or that's the first use case? Should the pins be something like a Pinnable list group?
Do we want to persist the order of the pins (in case they change the table order) in the url and make it sharable?

Answering your questions @jennypavlova here :) -
we want to persist the pinned table data in the order they are pinned - so yup, when the user clicks on the "pin", the row goes to the top and stays there: https://www.loom.com/share/37683cef774e4a318fa15ccbc63b24c4

As for the component and if it's the first time using it - So it is present on 'Analytics' - Discover. I think the closest thing in the EUI to this is the 'adding actions' to the table' section (where btw is the "Actions" column from the other designs :)
https://elastic.github.io/eui/#/tabular-content/tables#adding-selection-to-a-table -->

About Do we want to persist the order of the pins (in case they change the table order) in the url and make it sharable?
What kind of efford would that be? I assume the pinning would be more of a 'personal' preference thing. The thing that would be great to share are of course the filters put on the table - with that it should suffice. @roshan-elastic mentioning you here FYI. What do you think? I think I would drop this in the share.

@roshan-elastic
Copy link

About Do we want to persist the order of the pins (in case they change the table order) in the url and make it sharable?
What kind of efford would that be? I assume the pinning would be more of a 'personal' preference thing. The thing that would be great to share are of course the filters put on the table - with that it should suffice. @roshan-elastic mentioning you here FYI. What do you think? I think I would drop this in the share.

Hey @kkurstak / @jennypavlova - I think we share the filters in the table, just not the pinning (pinning would be great but, assuming that the pins are stored in local storage, we don't want to override another user's saved pins by opening a URL).

Make sense?

Food for thought...

BTW we should start thinking ahead (not in this issue - just in general) about what fly-outs would look like if they were the same everywhere in the UI (i.e. have one consistent fly-out for a host no matter where it is opened from).

Note : We may decide this fly-out should change depending on the context (or give the Elastic engineer, who wants to add a fly-out, a choice over what should render)

@jennypavlova jennypavlova self-assigned this Apr 25, 2023
@jennypavlova
Copy link
Member Author

jennypavlova commented Apr 26, 2023

@kkurstak @roshan-elastic Thanks for the comments!

we want to persist the pinned table data in the order they are pinned - so yup, when the user clicks on the "pin", the row goes to the top and stays there:

I am wondering if we want to have the alphabetical order like in Discover or if we want to persist in the current order (host -> cloud -> agent). I experimented a bit and I think the current order looks good but want to hear your opinion. The background around is that we already have the metadata sorted in a certain way and if we introduce different sorting can be confusing (the lack of sorting and just moving the pins can look also messy) so I did something like:

Screen.Recording.2023-04-26.at.13.59.50.mov

That way the agent.* pin will go back to the original order if unpinned and will go after the host.* pins.
I was thinking in general that the scenario will be if you want to see first some of the bottom properties (like agent and cloud properties first) - but also we don't want to lose the original structure. I don't want to overcomplicate it and also don't want to lose the original view so if we still want to use alphabetical order (so agent -> cloud -> host ) for the pins I can add it (it's just that the current order/grouping is not alphabetical so I think it won't make sense). Of course inside host/agent/clould the alphabetical order will be persisted (if you pin both agent.version and agent.id- agent id will go first) Another example (you can see when you have only agent.* pins they are still ordered alphabetically and when you add host.* pins they will go first but won't lose the alphabetical order):

Screen.Recording.2023-04-26.at.14.23.25.mov

Edit: Example with cloud metadata:

Image

PS: Ignore the Add/Remove pin tooltips in the videos - the tooltips are Pin Field/Unpin field like in Discover it was the WIP state

What do you think about this order, does it make sense?

@jennypavlova
Copy link
Member Author

As discussed with @roshan-elastic we are closing this for now as we will probably replace the flyout with an embeddable in 8.9. I will keep the branch and close the issue and the PR for now.

@smith
Copy link
Contributor

smith commented Jun 27, 2023

Reopening this to be re-included in some host flyout updates.

@smith smith reopened this Jun 27, 2023
@jennypavlova
Copy link
Member Author

I will adapt the solution there to the new asset details embeddable ( using the closed PR and opening a new one once it's done).

@jennypavlova jennypavlova changed the title [Infrastructure UI] Hosts Flyout: Add pins to the metadata table [Infrastructure UI] Asset Details: Add pins to the metadata table Jul 3, 2023
jennypavlova added a commit that referenced this issue Jul 6, 2023
…61074)

Closes #155190

## Summary

This PR adds the possibility to pin different rows inside the metadata
table in asset details embeddable. The pins are persisted in the local
storage and should be available after refreshing/reopening the host
flyout. The order and sorting are explained in [this
comment](#155190 (comment)),
so basically we keep the original sorting order of the table (`host`,
`cloud`, `agent`) also for the pins.

## Testing 
- Go to hosts view and open a single host flyout (metadata tab)
- Try to add / remove pins
- Check if the pins are persisted after a page refresh



https://github.com/elastic/kibana/assets/14139027/62873e7e-b5f0-444c-94ff-5e19f2f46f58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Required for a feature to move to beta Feature:Metrics UI Metrics UI feature Feature:ObsHosts Hosts feature within Observability Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
5 participants