allegro / turnilo

Business intelligence, data exploration and visualization web application for Druid, formerly known as Swiv and Pivot
https://allegro.github.io/turnilo/
Apache License 2.0
729 stars 171 forks source link

Fullscreen mode (shareable by URL) / being able to hide FILTER, SPLIT, MEASURE tile. #543

Open KarolakJakub opened 4 years ago

KarolakJakub commented 4 years ago

Hey! We would like to be able to switch to a "fullscreen" mode - which would just mean being able to hide the top middle tile with filters (FilterTile, SplitTile, SeriesTilesRow, VisSelector) in the same way you can already do with side Pin and Dimensons panels. UI wise, it would just mean adding a functional ToggleArrow component below the measure tile.

image Harsh mockup.

Then, with a share URL available, one could embed it in a HTML iframe tag or just forward it in this form as it looks neater and would stop any unintentional changes in filters (misclicks etc.).

Main issue with this would come from Turnilo storing "visual" state and size of panels in the local storage. So we would probably have to add a flag to URL params which would force an override of local storage settings if present.

We would need a new share option which would include visual state of the application and differentiate it from the regular share links that wouldn't override user's local storage settings, or just have something like "Copy URL to fullscreen version". Any thoughts? :)

adrianmroz commented 4 years ago

I'm quite sure this is wrong solution for a problem. Tiles with filters, splits and measures should be always visible. They hold all information to decipher visualisation. (With asterisk for line chart legend which is designed poorly at the moment, so let's skip it).

Left and right panel are really menus/controls. They are shortcuts for user to manipulate visualisation so they should be defined by user, not by url. If we wan't to move it from local storage - server state is only sensible place, but we want to keep turnilo stateless.

What you want is different view altogether. You could add another route and present there only single, visualisation component. Things to consider: 1). What about interactivity? It should be mostly disabled but how to achieve that? 2). Ensure that all information is presented/annotated. It is still turnilo view, user should be able to understand visualisation under link without external information. (Of course that's nice to have, because I see that you want to sneak dashboards into turnilo :D)

mkuthan commented 4 years ago

We would like to be able to switch to a "fullscreen" mode - which would just mean being able to hide the top middle tile with filters (FilterTile, SplitTile, SeriesTilesRow, VisSelector) in the same way you can already do with side Pin and Dimensons panels. UI wise, it would just mean adding a functional ToggleArrow component below the measure tile.

Agree with @adrianmroz, top panel is much more important than left/right menus. But I also agree that full screen mode for embedding purposes is reasonable.

image Harsh mockup.

Nice, consistent design! But a few precious vertical pixels are lost.

Then, with a share URL available, one could embed it in a HTML iframe tag or just forward it in this form as it looks neater and would stop any unintentional changes in filters (misclicks etc.).

Hmm, if we want to stop any unintentional changes, hiding the panels is not enough. For iframe embedding purposes any components with colapse/expand options will be harmful.

Main issue with this would come from Turnilo storing "visual" state and size of panels in the local storage. So we would probably have to add a flag to URL params which would force an override of local storage settings if present.

There are at least two reasons to keep visual state locally:

  1. Right / left menus appearance is a specific user preference shared across all visualisation. If user does not use right menu, he/she hides that menu and this is a private preference of that user.
  2. Screen resolutions are different. It does not make sense to share information like right menu size: 100px.
  3. Max URL length is limited

We would need a new share option which would include visual state of the application and differentiate it from the regular share links that wouldn't override user's local storage settings, or just have something like "Copy URL to fullscreen version". Any thoughts? :)

mkuthan commented 4 years ago

I'm quite sure this is wrong solution for a problem. Tiles with filters, splits and measures should be always visible. They hold all information to decipher visualisation. (With asterisk for line chart legend which is designed poorly at the moment, so let's skip it).

I fully agree, presenting reports without important context data (like current filters) will be misleading and harmful.

Left and right panel are really menus/controls. They are shortcuts for user to manipulate visualisation so they should be defined by user, not by url. If we wan't to move it from local storage - server state is only sensible place, but we want to keep turnilo stateless.

What you want is different view altogether. You could add another route and present there only single, visualisation component. Things to consider: 1). What about interactivity? It should be mostly disabled but how to achieve that? 2). Ensure that all information is presented/annotated. It is still turnilo view, user should be able to understand visualisation under link without external information. (Of course that's nice to have, because I see that you want to sneak dashboards into turnilo :D)

I would think about "read only" mode rather than fullscreen. In this mode the following UI modification could be applied:

I would decode this mode in the view definition as boolean flag. It should not be easy to hack and change the mode by editing URL. But I'm open for other proposals.

adrianmroz commented 4 years ago

I would decode this mode in the view definition as boolean flag. It should not be easy to hack and change the mode by editing URL. But I'm open for other proposals.

I think different route would be better. I don't see why it shouldn't be obvious for user to "hack" one version to another.

It could also would look better in code - different components instead of bunch of nested conditionals.

mkuthan commented 4 years ago

I would decode this mode in the view definition as boolean flag. It should not be easy to hack and change the mode by editing URL. But I'm open for other proposals.

I think different route would be better. I don't see why it shouldn't be obvious for user to "hack" one version to another.

It could also would look better in code - different components instead of bunch of nested conditionals.

Perhaps you are right, the same comment for read only versions of header / filter / split / measure components. Separate components instead of bunch of conditionals in the existing ones.

KarolakJakub commented 4 years ago

Thank you for feedback! Read-only mode does sound like a way better and neater solution.

if author define report refresh interval it also should be presented read-only on the header

Currently, refresh options are stored in the application state and cannot be shared. Do we want to change that?

For the URL, if the 'read only' mode doesn't have to be hidden from the user, would something like this work?

http://localhost:9090/#data_cube_name/read-only/rest of the url ....

KarolakJakub commented 4 years ago

Also, just to clarify, we want some conditionals in the cube-view.tsx that would render regular components or their 'read-only' counterparts, based on the current URL? The alternative is to have a 'read-only' variant of cube-view.tsx which would extend the regular CubeView class.

adrianmroz commented 4 years ago

http://localhost:9090/#data_cube_name/read-only/rest of the url ....

/#read-only/data_cube_name/hash - we want new route

The alternative is to have a 'read-only' variant of cube-view.tsx which would extend the regular CubeView class.

new route, new component. composition, no inheritance

AlejandroITOP commented 4 years ago

Are these contributions currently implemented?

mkuthan commented 4 years ago

Currently, refresh options are stored in the application state and cannot be shared. Do we want to change that?

Good point, it could be a part of view definition. Or maybe this is some kind of user preferences, I don't know :(