search

sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.12k stars 1.29k forks source link

Code Insights [spike]: Keep order and size of insight cards on the dashboard page #29536

Closed vovakulikov closed 2 years ago

vovakulikov commented 2 years ago

Related product board item https://sourcegraph.productboard.com/roadmap/3362047-code-insights-beta-to-ga/features/9370719/detail/expanded

The problem

At the moment we don't have any system over saving size and order for cards items on the dashboard page. There are a few things we should resolve before diving deep into the implementation of it:

Product and API questions

  1. Ordering (GQL API mutation to change the ordering of insight in a dashboard)
  2. Does the ordering affect pagination in the existing queries for dashboard insights? Probably yes is it a problem? Do we need to have an absolute order of insight by default?
  3. What is the system over saving order and size on the dashboard page? Is it only a dashboard attribute or different users can have different dashboards (with different card sizes and ordering) and switch on/off their personal setting to kind of "global" dashboard settings for sizing/ordering? Like GH dashboards for example cc @AlicjaSuska @Joelkw
  4. If users can have their personal settings over the dashboard who can modify "global" dashboard settings in this case? only admins?

Tech questions

  1. I remember that there are a few problems with react-grid-layout if we need to save and persist something. Do we need to consider moving away from react-grid-layout lib now or is it possible to implement persistent order size with that solution for now?
Joelkw commented 2 years ago

@vovakulikov Thank you for these thoughts! I'm numbering the responses to correspond to the points they refer to.

  1. @vovakulikov could you record a brief loom of how GH does it? My default assumption based on the dozen dataviz tools I've used is that we do not need to support per-user ordering and sizing – everyone can see the same dashboard, in the same chart order, with the charts the same respective size when viewed by any user. (This is how looker, amplitude, google analytics, and sisense periscope data function. Is grafana different cc @coury-clark ?)
  2. I'm not sure I understand this situation, but generally – for global dashboards, for now (until we get eventual requests from customers to build new permissions models, if they need) I would assume anyone can edit order and size (but not unintentionally – there should be a clear 'save' state).
AlicjaSuska commented 2 years ago

Thank you for putting this together @vovakulikov! @Joelkw I have a different point of view when it comes to global vs. per-user settings. In my opinion, we should save the layout per user. My reasoning:

Our goals:

If we save layout per user: each person experiences the default layout first, they can make changes and during their next visit their experience is improved and predictable. Goals are met.

If we save layout per dashboard: we achieve the opposite - every time user comes back to the dashboard they may see a completely different layout and order. Given that all users would have the right to modify the layout, it will probably happen very often (on purpose or not). In addition, users would probably assume that they make modifications only for themselves, not for everyone viewing the dashboard. It may cause a lot of mess (especially when people are just playing with the resizing and reordering) and, in consequence, fear to explore insights in the future.

Default layout and admin rights: for minimal version, I think that default layout should be provided by us, not by admins. Maybe in the future, if requested, we can add this feature. I definitely see the value in letting admins provide a 'great default' but it seems excessive for the first iteration. That being said, if we can leave space for this improvement in the future, that would be great.

Thoughts about sharing dashboards with saved layouts - it is probably out of the scope but sharing here anyways to give a bigger picture of what could be useful / considered in the future:

What happens when user A shares a link to the dashboard with the user B? Cases:

  1. User B has modified the dashboard in the past - we should display user B saved settings
  2. User B hasn't modified the dashboard in the past - should we display the default layout or apply user A layout? My take: Given that we don't know if user B is familiar with the dashboard and simply likes the default layout or they haven't seen the dashboard before, it would be risky to apply user A settings permanently for user B. In a perfect world:
    • layout settings would be connected to the shared URL
    • if user B has custom settings saved for the given dashboard: we should apply user B settings, ignore user A layout.
    • if user B has no custom settings saved for the given dashboard: we should apply user A layout when opening the dashboard from the link but not set the new default for the user B (possibly give user B ability to save this layout manually). If user B opens the same dashboard from the dropdown, we should display the default layout.

Why: It would provide the most consistent experience when sharing and discussing the dashboard in real-life situations.

That being said, we can definitely skip this logic for now and wait for the feedback in the future.

coury-clark commented 2 years ago

@vovakulikov Thank you for these thoughts! I'm numbering the responses to correspond to the points they refer to.

3. @vovakulikov could you record a brief loom of how GH does it? My default assumption based on the dozen dataviz tools I've used is that we do not need to support per-user ordering and sizing – everyone can see the same dashboard, in the same chart order, with the charts the same respective size when viewed by any user. (This is how looker, amplitude, google analytics, and sisense periscope data function. Is grafana different cc @coury-clark ?)

4. I'm not sure I understand this situation, but generally – for global dashboards, for now (until we get eventual requests from customers to build new permissions models, if they need) I would assume anyone can edit order and size (but not unintentionally – there should be a clear 'save' state).

Grafana behaves such that ordering settings are coupled to the dashboard, and everyone will see the same thing if they load the saved version. Local changes (unsaved) are client-side until they are saved.

I'm split mind on this, and would like to give it more thought today. My first instinct was also that we should copy other tools and have dashboards be coupled to ordering, but something about the arguments in favor of per-user ordering from @AlicjaSuska I find compelling.

Joelkw commented 2 years ago

@AlicjaSuska Thank you for the thoughts! I still have a difference of opinion, but will try to explain with some in-line responses and three other points. If we need to, we can discuss more in the EM-PM-Design sync or ad-hoc sync:

Our goals:

  1. improve the readability
  2. provide consistent experience for the user (not for the group) when viewing the dashboard
  3. make the dashboard easier to consume ('muscle memory')

^ I Agree. But would note that bullet #2 is equally well-served by dashboard-saved (rather than user-saved) settings, because it's still consistent for the user. In fact, you could argue it's more consistent – you never wonder if someone else is looking at a different chart when they say "the top right one" or similar.

If you need a dashboard that is very different from what other people need, you can always create your own. I find it common that people do create their own dashboards to solve the problem of "I want to have this dashboard, but minus the stuff I don't care about." But you wouldn't modify the "main" dashboard just for yourself if you want to know exactly how other people see/consume and what they refer to in conversation or in reference as well ("no, the graph at the very bottom" etc).

If we save layout per user: each person experiences the default layout first, they can make changes and during their next visit their experience is improved and predictable. Goals are met.

It feels to me less predictable that your reordering changes are local, since most data products don't do this (waiting on the GH example). More on this further down – would love other examples.

If we save layout per dashboard: we achieve the opposite - every time user comes back to the dashboard they may see a completely different layout and order.

This is possible but unlikely (right now, anyone can edit the sets of insights on dashboards and this hasn't been a problem) – and assuming we build it so that you have to explicitly save, and we communicate "this is going to change the layout for everyone else too."

Given that all users would have the right to modify the layout, it will probably happen very often (on purpose or not).

(See prior note on how we should make it not save unless you explicitly save, the way filters work in the product too.)

In addition, users would probably assume that they make modifications only for themselves, not for everyone viewing the dashboard.

I'd love to dig into this assumption more! Are there products you use that feel similar? It sounds like both in @coury-clark and in my experience with data tools, "saving" means everyone sees the same thing, not that it's per-user.

Additional considerations

1) User saved settings gets complicated very fast: You went into a couple already in your example. This could cause us to spend a lot of time on a feature that may not unlock tremendous value, even if it was the marginally better choice – the "good enough" of ordering/sizing options plus the "excellent" ability to make powerful charts to put on the dashboards is the value of Code Insights (we know this to be true from our overall prioritization investigations – for example, that's why capture group charts came before this feature). And if there was only one ordering + view for dashboards, we'd avoid this confusion.

3) Dashboard-saved settings better serve a common use case, where one person makes dashboards explicitly for others: it is common that a staff SWE, or even a team of centralized data folks, make dashboards for a high-ranking engineering leader or other team. In that case, they will always want the people they make dashboards for see the same view, because the "consumers" themselves aren't taking time to learn the product (or care to make modifications – they would just ask the main team). If the creators wanted to to reorder charts to refine them before sharing, if we do user-saved settings we now also have to build an explicit "share link" flow, but if we use dashboard-saved settings they just share the url and be confident their refinements are shared.

2) We have some user feedback about "locking down changes" but it's not related to per-user views: We don't have a ton of user feedback to "user-saved" vs "dashboard-saved" orderings. Depending on folks responses, I am happy to go solicit some quick takes from users now. What we do have – which is separate – is feedback that longer term, we may need to lock down "editing" permissions in certain ways. But this is not the same as "do user-saved settings" because in that case, folks still want everyone to see the same dashboard (not just themselves, but also their org or exec team, etc – so they'd have to lock down settings in either the user-saved or dashboard-saved case to guarantee consistent views for all).

Joelkw commented 2 years ago

@vovakulikov sorry if I missed this product question:

Do we need to have an absolute order of insight by default?

Yes, dashboards should (ideally, we can prioritize this separately if it's a problem) load the same ordering every time you visit them. We can make exceptions explicitly (show "newest" insights at the top, for example). If we had any saved settings model, though, I assume we'd get this?

github-actions[bot] commented 2 years ago

Heads up @joelkw @felixfbecker @vovakulikov @unclejustin - the "team/code-insights" label was applied to this issue.

coury-clark commented 2 years ago

@Joelkw I've given this some more thought. My opinion is that we should preserve ordering and layout per-dashboard. I do feel that something about our product is subtly different (I can't quite pinpoint it) where I could actually imagine per-user configurations making sense; however, I think it's a big complication that doesn't stand out to me as substantially different than separate dashboards. I think we could accomplish similar goals with a copy (duplicate) dashboard function.

Having said that, I do see some reason to believe that non-persisted changes to layout and order could be useful client-side (imagine arranging a screenshot, or something similar). Once again though, this is trivially solved with creating a new dashboard and adding the insights to that dashboard.

Joelkw commented 2 years ago

I do see some reason to believe that non-persisted changes to layout and order could be useful client-side (imagine arranging a screenshot, or something similar)

I agree! I defer to @AlicjaSuska but I feel like it should be possible to edit on a "per session" basis and only save if you explicitly want to save the layout changes.

AlicjaSuska commented 2 years ago

Thank you for all the thoughts @Joelkw and @coury-clark. I see your points. In this case, let's move forward with:

I will post the updated designs in this thread soon.

vovakulikov commented 2 years ago

First of all thanks to everyone for your thoughts and different options.

The bottom line that I got from this thread is

First, we are going to have (at least for v.1 of this functionality) per-dashboard settings by explicit action (like click on save button) and notifying that this action's changes are dashboard's settings - so be gentle with that.

Second, we also allow user session settings to change the order and cards size. But if we update the dashboard we gonna lose these settings and apply the last saved dashboard settings.

Joelkw commented 2 years ago

Agree with this @vovakulikov . Would note that "user session settings" do not need to persist for v1 -- what we have now, where you can resize but they do not save, is acceptable, given you can persist by saving them to the dashboard.

AlicjaSuska commented 2 years ago

I've updated the design to accommodate for saving layout per dashboard. Please, see the Loom and comment directly in Figma.

I've also covered how the interface would look like with tabs (for the 'Getting started' page) and the 'Assign insights' button taken out of the three dots menu (improvement based on usability testing).

@Joelkw @vovakulikov

Joelkw commented 2 years ago

(Linking back – https://github.com/sourcegraph/sourcegraph/issues/29960 )

coury-clark commented 1 year ago

Just had +1 request from another customer who wants to save the resized insights

nihaals commented 1 year ago

I'm not sure if this is related to this issue or a bug but rearranging cards or changing the size doesn't save on either "All Insights" or on a specific dashboard. The reason I question if it's a recent bug is because not all my cards are the same size and I can't seem to find any way of doing this for new cards anymore so I assume this used to be possible. This is on v5.0.0.

coury-clark commented 1 year ago

@nihaals Hi there! Just for clarity, the dashboards have never saved either position or size. With that in mind, are you saying the cards are different sizes for you?

nihaals commented 1 year ago

@coury-clark Yes, some of my cards are normal squares (around 1/3 width of the page) while other cards are 1/2 width of the page, which I assumed I did myself.

coury-clark commented 1 year ago

@nihaals Got it! Understood - that's working as intended! It follows some rules like this:

If it's a capture group insight when it takes 1/2 width of the row
Other insight in the row (not exactly capture group insight) also becomes 1/2 width (just to avoid gaps in the layout)
If it's search-based insight with 4 and more series it takes 1/2 insight
Other insight in the same row also takes 1/2 width as it works with capture group insight
nihaals commented 1 year ago

I'd like to also +1 then as it not saving felt like a bug and it doesn't feel expected to be able to arrange them without it saving. It would also just be helpful as some of my dashboards have cards that are more of a general summary with other cards that dive into specific details and having the general ones at the top would make skimming the dashboard much nicer.

coury-clark commented 1 year ago

@nihaals Thank you for the feedback!