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

[Drilldowns] Dashboard broken back button if embeddable update input during initial rendering #61596

Closed
Dosant opened this issue Mar 27, 2020 · 4 comments · Fixed by #62415
Closed
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:Drilldowns Embeddable panel Drilldowns

Comments

@Dosant
Copy link
Contributor

Dosant commented Mar 27, 2020

While working on dashboard-to-dashboard drilldown a handful of issues were fixed to improve back button experience. #61230. But this particular one stands out.

Reproduce:

  1. Load logs sample data
  2. Open [Logs] Web Traffic dashboard
  3. Press back button

Expected behaviour: navigate back to listing page
Current behaviour (after all fixes from #61230 in master): nothing

If to look into history stack:

#/dashboard/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_a=(description:'Analyze%20mock%20web%20traffic%20log%20data%20for%20Elastic!'s%20website',filters:!(),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),panels:!((embeddableConfig:(vis:(colors:('Avg.%20Bytes':%236ED0E0,'Unique%20Visitors':%230A437C),legendOpen:!f)),gridData:(h:13,i:'2',w:21,x:27,y:11),id:e1d0f010-9ee7-11e7-8711-e7a007dcef99,panelIndex:'2',type:visualization,version:'8.0.0'),(embeddableConfig:(isLayerTOCOpen:!f),gridData:(h:18,i:'4',w:24,x:0,y:49),id:de71f4f0-1902-11e9-919b-ffe5949a18d2,panelIndex:'4',type:map,version:'8.0.0'),(embeddableConfig:(vis:(defaultColors:('0%20-%2022':'rgb(247,251,255)','22%20-%2044':'rgb(208,225,242)','44%20-%2066':'rgb(148,196,223)','66%20-%2088':'rgb(74,152,201)','88%20-%20110':'rgb(23,100,171)'),legendOpen:!f)),gridData:(h:13,i:'7',w:24,x:0,y:36),id:'935afa20-e0cd-11e7-9d07-1398ccfcefa3',panelIndex:'7',type:visualization,version:'8.0.0'),(embeddableConfig:(mapCenter:!(36.8092847020594,-96.94335937500001),vis:(params:(sort:(columnIndex:!n,direction:!n)))),gridData:(h:12,i:'9',w:21,x:27,y:24),id:'4eb6e500-e1c7-11e7-b6d5-4dc382ef7f5b',panelIndex:'9',type:visualization,version:'8.0.0'),(embeddableConfig:(title:'',vis:(colors:('0%20-%20500':%23BF1B00,'1000%20-%201500':%237EB26D,'500%20-%201000':%23F2C96D),defaultColors:('0%20-%20500':'rgb(165,0,38)','1000%20-%201500':'rgb(0,104,55)','500%20-%201000':'rgb(255,255,190)'),legendOpen:!f)),gridData:(h:11,i:'11',w:9,x:10,y:0),id:'69a34b00-9ee8-11e7-8711-e7a007dcef99',panelIndex:'11',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:12,i:'13',w:27,x:0,y:24),id:'42b997f0-0c26-11e8-b0ec-3bb475f6b6ff',panelIndex:'13',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:31,i:'14',w:24,x:24,y:36),id:'7cbd2350-2223-11e8-b802-5bcf64c2cfb4',panelIndex:'14',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:13,i:'15',w:27,x:0,y:11),id:'314c6f60-2224-11e8-b802-5bcf64c2cfb4',panelIndex:'15',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'16',w:15,x:19,y:0),id:'24a3e970-4257-11e8-b3aa-73fdaf54bfc9',panelIndex:'16',type:visualization,version:'8.0.0'),(embeddableConfig:(vis:(legendOpen:!f)),gridData:(h:11,i:'17',w:14,x:34,y:0),id:'14e2e710-4258-11e8-b3aa-73fdaf54bfc9',panelIndex:'17',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'18',w:10,x:0,y:0),id:'47f2c680-a6e3-11e8-94b4-c30c0228351b',panelIndex:'18',type:visualization,version:'8.0.0')),query:(language:kuery,query:''),timeRestore:!t,title:'%5BLogs%5D%20Web%20Traffic',viewMode:view)&_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-7d,to:now))
#/dashboard/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_a=(description:'Analyze%20mock%20web%20traffic%20log%20data%20for%20Elastic!'s%20website',filters:!(),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),panels:!((embeddableConfig:(vis:(colors:('Avg.%20Bytes':%236ED0E0,'Unique%20Visitors':%230A437C),legendOpen:!f)),gridData:(h:13,i:'2',w:21,x:27,y:11),id:e1d0f010-9ee7-11e7-8711-e7a007dcef99,panelIndex:'2',type:visualization,version:'8.0.0'),(embeddableConfig:(isLayerTOCOpen:!f,mapCenter:(lat:42.16337,lon:-88.92107,zoom:3.64)),gridData:(h:18,i:'4',w:24,x:0,y:49),id:de71f4f0-1902-11e9-919b-ffe5949a18d2,panelIndex:'4',type:map,version:'8.0.0'),(embeddableConfig:(vis:(defaultColors:('0%20-%2022':'rgb(247,251,255)','22%20-%2044':'rgb(208,225,242)','44%20-%2066':'rgb(148,196,223)','66%20-%2088':'rgb(74,152,201)','88%20-%20110':'rgb(23,100,171)'),legendOpen:!f)),gridData:(h:13,i:'7',w:24,x:0,y:36),id:'935afa20-e0cd-11e7-9d07-1398ccfcefa3',panelIndex:'7',type:visualization,version:'8.0.0'),(embeddableConfig:(mapCenter:!(36.8092847020594,-96.94335937500001),vis:(params:(sort:(columnIndex:!n,direction:!n)))),gridData:(h:12,i:'9',w:21,x:27,y:24),id:'4eb6e500-e1c7-11e7-b6d5-4dc382ef7f5b',panelIndex:'9',type:visualization,version:'8.0.0'),(embeddableConfig:(title:'',vis:(colors:('0%20-%20500':%23BF1B00,'1000%20-%201500':%237EB26D,'500%20-%201000':%23F2C96D),defaultColors:('0%20-%20500':'rgb(165,0,38)','1000%20-%201500':'rgb(0,104,55)','500%20-%201000':'rgb(255,255,190)'),legendOpen:!f)),gridData:(h:11,i:'11',w:9,x:10,y:0),id:'69a34b00-9ee8-11e7-8711-e7a007dcef99',panelIndex:'11',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:12,i:'13',w:27,x:0,y:24),id:'42b997f0-0c26-11e8-b0ec-3bb475f6b6ff',panelIndex:'13',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:31,i:'14',w:24,x:24,y:36),id:'7cbd2350-2223-11e8-b802-5bcf64c2cfb4',panelIndex:'14',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:13,i:'15',w:27,x:0,y:11),id:'314c6f60-2224-11e8-b802-5bcf64c2cfb4',panelIndex:'15',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'16',w:15,x:19,y:0),id:'24a3e970-4257-11e8-b3aa-73fdaf54bfc9',panelIndex:'16',type:visualization,version:'8.0.0'),(embeddableConfig:(vis:(legendOpen:!f)),gridData:(h:11,i:'17',w:14,x:34,y:0),id:'14e2e710-4258-11e8-b3aa-73fdaf54bfc9',panelIndex:'17',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'18',w:10,x:0,y:0),id:'47f2c680-a6e3-11e8-94b4-c30c0228351b',panelIndex:'18',type:visualization,version:'8.0.0')),query:(language:kuery,query:''),timeRestore:!t,title:'%5BLogs%5D%20Web%20Traffic',viewMode:view)&_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-7d,to:now))
#/dashboard/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_a=(description:'Analyze%20mock%20web%20traffic%20log%20data%20for%20Elastic!'s%20website',filters:!(),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),panels:!((embeddableConfig:(vis:(colors:('Avg.%20Bytes':%236ED0E0,'Unique%20Visitors':%230A437C),legendOpen:!f)),gridData:(h:13,i:'2',w:21,x:27,y:11),id:e1d0f010-9ee7-11e7-8711-e7a007dcef99,panelIndex:'2',type:visualization,version:'8.0.0'),(embeddableConfig:(hiddenLayers:!(),isLayerTOCOpen:!f,mapCenter:(lat:42.16337,lon:-88.92107,zoom:3.64),openTOCDetails:!()),gridData:(h:18,i:'4',w:24,x:0,y:49),id:de71f4f0-1902-11e9-919b-ffe5949a18d2,panelIndex:'4',type:map,version:'8.0.0'),(embeddableConfig:(vis:(defaultColors:('0%20-%2022':'rgb(247,251,255)','22%20-%2044':'rgb(208,225,242)','44%20-%2066':'rgb(148,196,223)','66%20-%2088':'rgb(74,152,201)','88%20-%20110':'rgb(23,100,171)'),legendOpen:!f)),gridData:(h:13,i:'7',w:24,x:0,y:36),id:'935afa20-e0cd-11e7-9d07-1398ccfcefa3',panelIndex:'7',type:visualization,version:'8.0.0'),(embeddableConfig:(mapCenter:!(36.8092847020594,-96.94335937500001),vis:(params:(sort:(columnIndex:!n,direction:!n)))),gridData:(h:12,i:'9',w:21,x:27,y:24),id:'4eb6e500-e1c7-11e7-b6d5-4dc382ef7f5b',panelIndex:'9',type:visualization,version:'8.0.0'),(embeddableConfig:(title:'',vis:(colors:('0%20-%20500':%23BF1B00,'1000%20-%201500':%237EB26D,'500%20-%201000':%23F2C96D),defaultColors:('0%20-%20500':'rgb(165,0,38)','1000%20-%201500':'rgb(0,104,55)','500%20-%201000':'rgb(255,255,190)'),legendOpen:!f)),gridData:(h:11,i:'11',w:9,x:10,y:0),id:'69a34b00-9ee8-11e7-8711-e7a007dcef99',panelIndex:'11',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:12,i:'13',w:27,x:0,y:24),id:'42b997f0-0c26-11e8-b0ec-3bb475f6b6ff',panelIndex:'13',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:31,i:'14',w:24,x:24,y:36),id:'7cbd2350-2223-11e8-b802-5bcf64c2cfb4',panelIndex:'14',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:13,i:'15',w:27,x:0,y:11),id:'314c6f60-2224-11e8-b802-5bcf64c2cfb4',panelIndex:'15',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'16',w:15,x:19,y:0),id:'24a3e970-4257-11e8-b3aa-73fdaf54bfc9',panelIndex:'16',type:visualization,version:'8.0.0'),(embeddableConfig:(vis:(legendOpen:!f)),gridData:(h:11,i:'17',w:14,x:34,y:0),id:'14e2e710-4258-11e8-b3aa-73fdaf54bfc9',panelIndex:'17',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'18',w:10,x:0,y:0),id:'47f2c680-a6e3-11e8-94b4-c30c0228351b',panelIndex:'18',type:visualization,version:'8.0.0')),query:(language:kuery,query:''),timeRestore:!t,title:'%5BLogs%5D%20Web%20Traffic',viewMode:view)&_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-7d,to:now))

Redundant changes are triggered by maps embeddable:

(embeddableConfig:(isLayerTOCOpen:!f)
(embeddableConfig:(isLayerTOCOpen:!f,mapCenter:(lat:42.16337,lon:-88.92107,zoom:3.64))
(embeddableConfig:(hiddenLayers:!(),isLayerTOCOpen:!f,mapCenter:(lat:42.16337,lon:-88.92107,zoom:3.64)

I assume what is happening, is when map embeddable from older kibana version is opened, maps are doing state migrations on a fly and update input during initial rendering.

https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/maps/public/embeddable/map_embeddable.tsx#L190

I am not sure, if there are other cases like this except for maps. Didn't find in dashboards from sample data

Please Note:

  1. If to Save Dashboard now and return to it - history button works correctly, because we saved last state which map embeddable expects!

  2. This maps behaviour causing other issue: as it updates input multiple time, it will retrigger redundant rerenders for other panels

This is not only migration edge case

  1. Create a new map
  2. Go to dashboard, create new dashboard
  3. Add this map do dashboard
  4. Press back button

The same problem is happening, app adds new fields to embeddable state during it's initial rendering.

Possible solutions:

  1. Fix in maps. ? Is this possible?
    2. Come up with some magic "all settled" on parent embeddable? start syncing state in dashboard when it emitted? - won't work because issue is actually happening not only during initial rendering
    3. Magic setTimeout in dashboard before push state into url? - won't work because issue is actually happening not only during initial rendering
  2. Consider not storing all the state in the URL?

cc @stacey-gammon @ppisljar @streamich

p.s.: maps are ignoring most of state changes which are coming from embeddable input, so history button almost not working for user actions done in maps. but this seems like separate bug

@Dosant Dosant added bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Team:AppArch Feature:Drilldowns Embeddable panel Drilldowns labels Mar 27, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@ppisljar
Copy link
Member

another possible solution would be moving the logic into migration script or (preferably not as we need to move that one to migration script as well) move it into visualizations/legacy/vis_update_state

@Dosant
Copy link
Contributor Author

Dosant commented Apr 1, 2020

@ppisljar, just realised that this is not only initial migration issue. updated the description:

Create a new map
Go to dashboard, create new dashboard
Add this map do dashboard
Press back button

And, basically, the same thing is happening when opening older dashboard for the first time

@Dosant
Copy link
Contributor Author

Dosant commented Apr 3, 2020

Agreed to try to solve this by removing panels state from the url in view mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:Drilldowns Embeddable panel Drilldowns
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants