feat: adds summary tray to service list view

[kleinfreund]

Changes

feat(services): adds summary tray

Adds a summary tray to the service list view.

Resolves #1609.

Signed-off-by: Philipp Rudloff [email protected]

Notes

• Follow up to feat: adds row selection ability to app collection #1601, refactor: avoids duplicating title bar template #1604 and feat: adds tray view #1613.

Screenshots

[netlify]

✅ Deploy Preview for kuma-gui ready!

Name	Link
🔨 Latest commit	8737f63
🔍 Latest deploy log	https://app.netlify.com/sites/kuma-gui/deploys/6537b9e6c47dbb0008861c84
😎 Deploy Preview	https://deploy-preview-1611--kuma-gui.netlify.app/gui
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[johncowen]

Couple of inlines/q's:

[johncowen]

Ok based on some of the conversations above I had a quick go at a potentially throw away branch/commit ontop of here to satisfy my curiosity:

dfdd7a8

Additions:

• The sub route/view uses all the same patterns we already have in the application for consistency and reuse:
  • RouteView/RouteTitle: route utilities and titling
  • AppView: headers/titling
  • stack and KCard: layout and spacing
• Kinda personal opinion that I'd be happy to change if more people in the team were against, but I would rather that when I click off the tray that it should close. So I removed the preventCloseOnBlur. I'm also guessing that it defaults to false because the closing behaviour is probably the preferred option.
• I used :is in the CSS to:
  • Make sure we can keep using <style scoped>
  • Remove the !importants
  • In chatting to you I realized just how coupled the 'tray' was to AppCollection, so I renamed it to AppCollectionSummary. I'm pretty sure I like this, wdyt?

I think overall the best thing about this is: Codewise, the summary route component looks exactly the same as all our other route components, it uses the same components, the same code and has the same functionality as all our other routes.

(P.S. I did take a quick look at view re-use if we ever wanted to do that, but I didn't bother to PR that here seeing as we are currently unsure if we want to do that)

Let me know what you think.

[johncowen]

Separate to all the conversation here on the code:

Functionally if I enter say the following URL:

https://deploy-preview-1611--kuma-gui.netlify.app/gui/meshes/matrix-0/services/johns-service?page=2&size=50

And johns-service has moved to page 1 because I've removed a bunch of services, I should still see page 2 of the list with the panel opened on johns-service, even if johns-service isn't in the list I'm looking at.

[kleinfreund]

And johns-service has moved to page 1 because I've removed a bunch of services, I should still see page 2 of the list

That’s happening.

with the panel opened on johns-service, even if johns-service isn't in the list I'm looking at.

Let’s discuss with the team whether this should be the case. When I spoke with Charly, we were thinking the URL should change to the plain list view (i.e. without /johns-service) or to show some sort of messaging explaining that the service wasn’t found, but this wasn’t completely conclusive.

[johncowen]

or to show some sort of messaging explaining that the service wasn’t found

What's the idea there, then would I have to click through the pages of the list to re-find the service I'm looking for and then re-click on it even though its already in the URL?

[johncowen]

Thanks for using AppView here, I added a few more comments/questions but I'm going to take it for another spin now we've made a few more changes here, in the meantime let me know on those.

[johncowen]

Just wanted to say, that I gave this another spin and its looking great. I think we are very nearly there with this.

I think there are a few more outstanding comments, just wanted to say thanks for bearing with me for a lot of the comments here. As we are rolling this out everywhere I wanted to be as careful as I could with the code that is going to get repeated in lots of places.

Let me know on the outstanding comments (I think its just the RouteView::t, and questions around that extra div/spacing)

[johncowen]

What are we doing about what happens when a service isn't on the page I am viewing?

i.e. the

/meshes/default/services/johns-service?page=2 (with johns-service being on page 1)

[lahabana]

I'm sorry I don't have much time to look at what actually we're doing so I'll explain 2 theories:

1. We fetch data in the tray view (there's a Datasource in the tray). then let's still render with a list that doesn't contain the item
2. We don't fetch data then let's redirect to the list at the page without the tray (if possible with a banner explaining what's up). It's not amazing but I believe the only cases where I see this happening is when there has been changes in the list and we're either sharing the url (likely sharing the detail view url is better) or refreshing the page. I think both these cases are rare enough to sacrifice UX for code simplicity.

WDYT?

[johncowen]

We don't fetch data then let's redirect to the list at the page without the tray (if possible with a banner explaining what's up). It's not amazing but I believe the only cases where I see this happening is when there has been changes in the list and we're either sharing the url (likely sharing the detail view url is better) or refreshing the page. I think both these cases are rare enough to sacrifice UX for code simplicity.

We could only use the DataSource if there's not a props.data, i.e.

<DataSource
  :src="props.data ? `` : `/meshes/${route.params.mesh}/services/${route.params.service}`"
  v-slot="{ data }"
>
<template v-for="service in [props.data || data]">
{{service.name}}

but in saying that, I'm happy to leave this for the moment. I think you are right that its rare enough, and without something like a DataLoader we would also have to cope with errors and loading here meaning its simpler without. We can always change later.

---

Ok, so just to be clear from me, lets not do anything here on this point.

[kleinfreund]

I agree that we should consider how we handle this (both of Charly’s outlines need additional work anyway) outside of this PR. There might be situations where we need to DataSource within the trays (e.g. when the list data isn’t enough for the summary needs), too.

[johncowen]

Lets get this in!