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

[Visual Library] Non-modal editing of a Visual Library chart still offers Save and Return, lands in a Canvas workbook #92821

Closed
monfera opened this issue Feb 25, 2021 · 7 comments · Fixed by #92917
Labels
discuss Feature:Canvas Feature:Vislib Vislib chart implementation Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@monfera
Copy link
Contributor

monfera commented Feb 25, 2021

Kibana version:
7.12

Original install method (e.g. download page, yum, from source, etc.):
kbn bootstrap etc. from the 7.12 branch

Describe the bug:
as in the title

Steps to reproduce:

  1. Go to somewhere eg. home page
  2. Go to Visual Library
  3. "Save and Return" is shown, and actually lands you somewhere, like a forgotten continuation

Expected behavior:
Save and Return should only be active if we came here from within eg. a Canvas workpage where we initiated editing the chart

Screenshots (if relevant):
remember

Any additional context:
It's probably a hard problem to know if we're still in modal editing, eg. what if I forgot to ever return from an edit, then went to the home page. I think in this case there should be some reset: navigate away from the modal edit any way other than "Save and Return": clear the flag that allows modal return. Indefinite states shouldn't dangle around

@monfera monfera added Feature:Vislib Vislib chart implementation discuss Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Canvas labels Feb 25, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@flash1293
Copy link
Contributor

flash1293 commented Feb 25, 2021

I think this should be fixed by #91005 , but maybe the PR missed your specific navigation path @monfera could you check whether you have this commit locally?

cc @ThomThomson

@monfera
Copy link
Contributor Author

monfera commented Feb 25, 2021

Thanks Joe, I didn't have this commit and a git pull on 7.12 didn't add it, so it may still need a backport. Hey at least the "before" state got tested!

@ThomThomson
Copy link
Contributor

ThomThomson commented Feb 25, 2021

Good find @monfera, thanks! This may actually be due to another issue - which should have been fixed by #90243. I will look into why it's not applying here.

@ThomThomson
Copy link
Contributor

Seems like both #91005 and #90243 were merged correctly into 7.12 - (back when 7.x was 7.12)

Interestingly, this issue seems like more of a design / UX consideration.

@kmartastic @ryankeairns

This earlier issue: #90221 describes a situation where editing a by value panel would put the state into the app link, which meant that leaving (going to home for instance), then navigating back via the Visualize Library link would put you back to editing by value. The main problem with this was that there was no Visualize Library breadcrumb, which made it really hard to tell that was where you were, or to navigate back to the listing page.

I fixed that by saving the listing page to the app link instead any time the app was editing or creating a by value panel. That fixed the most egregious parts of the issue, but I can definitely see how it could be strange to have the app link take you back to a state where you're linked to an originating app. I am leaning towards the following fix:

Redirect to Listing Page:
Extend the functionality from #90243 so that the app link points to the listing page any time there is an originating app.

  • This is a really easy implementation.
  • This makes the app links more consistent: Lens doesn't affect the Visualize Library app link at all, and Maps doesn't even use the URL tracker, so its app link always points to the listing page.
  • This means the history will work correctly, if you hit save and return from an editor, then hit the browser back button, you will still be linked to the app. But the minute you hit the Visualize Library app link the editor state will be cleared.

@ryankeairns
Copy link
Contributor

If I'm following the flow correctly...

  • I was in Dashboard
  • went out to edit in Visualize Library
  • jumped off to the home page and
  • am now going directly to Visualize Library.

If that is the flow, then my expectation would be to see the listing page. In my mind, the editing I was doing prior was in the context of the dashboard, so I wouldn't expect to land back in that editing flow unless I went back through the original dashboard.

Suspecting that is your proposed solution, I am in agreement :) Thanks for coming to my TED Talk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Canvas Feature:Vislib Vislib chart implementation Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants