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

Added dashboard standalone page #1429

Merged
merged 4 commits into from
Oct 26, 2016

Conversation

vera-liu
Copy link
Contributor

@vera-liu vera-liu commented Oct 25, 2016

Feature request: #1354

Done:

  • When standalone=true is specified in url, a standalone dashboard page is rendered. This page doesn't have caravel header or button groups/fav star, it will be more convenient to make screenshots.

    without standalone arg:
    screen shot 2016-10-25 at 12 35 50 pm

    with standalone arg:
    screen shot 2016-10-25 at 12 36 42 pm

Todo:
I'm thinking about adding a button to our button group in dashboard view for getting standalone endpoint for dashboard, would this be useful?

needs-review @ascott @bkyryliuk @mistercrunch

@@ -1793,6 +1793,12 @@ def dashboard(self, dashboard_id):
def dashboard(**kwargs): # noqa
pass
dashboard(dashboard_id=dash.id)
if request.args.get("standalone") == "true":
return self.render_template(
"caravel/dashboard_standalone.html",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to render a new template for the standalone view? or could we add some logic to basic.html where if we are on the dashboard page and the standalone arg is true, don't show the header. we are doing something similar for the standalone viz's afaik.

https://github.com/ascott/caravel/blob/master/caravel/templates/caravel/basic.html#L24-L25

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that we have a separate standalone template for explore view: https://github.com/ascott/caravel/blob/f837733d858643152304469c899c096e31d2bf24/caravel/views.py#L1194

Apart from the header, I also took out the button groups, fav star to limit interactions on dashboard standalone page. We could have if statements in both basic.html and dashboard.html for rendering specific parts based on standalone argument, but I was thinking that having a separate template would be cleaner?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think keeping one template for the dashboard and adding the conditions where needed will be more clear in the long run since we won't have to maintain 2 templates. the conditional for show/hiding the header should work the same way in basic.html for both the slice and dashboard view, so we will probably only need the conditionals in the dashboard template.

@ascott
Copy link
Contributor

ascott commented Oct 25, 2016

cc @johnbodley

@mistercrunch
Copy link
Member

I vote for parameterizing the template we have now.

@vera-liu vera-liu force-pushed the vliu_dashboard_standalone branch 3 times, most recently from 3d68f35 to bdd6a11 Compare October 26, 2016 00:26
<button type="button" class="btn btn-default" data-dismiss="modal">
Close
</button>
{% if not dash_standalone %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this only needs to wrap the #add-slice-container element, since the modal will only be shown if the button that triggers it clicked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done:)

@@ -21,7 +21,7 @@

<body>
{% block navbar %}
{% if not viz or viz.request.args.get("standalone") != "true" %}
{% if not dash_standalone and (not viz or viz.request.args.get("standalone") != "true") %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this should be if not dash_standalone or (not rather than and

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change it to 'or', when dash_standalone=true, not dash_standalone = false, but viz is not passed in, so not viz will be true, which makes the if statement to be true and header is displayed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could also have a more generic "show_navbar" argument and pass it explicitly in both views.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we hide the navbar in any other cases besides explore-standalone? I think in views.py we render standalone.html when standalone=true in the url. Seems like {% if not viz or viz.request.args.get("standalone") != "true" %} will always be true, since standalone.html doesn't extend basic.html

@ascott
Copy link
Contributor

ascott commented Oct 26, 2016

last comment re: and/or. with that fix LGTM!

@ascott
Copy link
Contributor

ascott commented Oct 26, 2016

LGTM! 🚢 👍

@vera-liu vera-liu merged commit 64d1964 into apache:master Oct 26, 2016
@vera-liu vera-liu deleted the vliu_dashboard_standalone branch November 1, 2016 19:00
@github35
Copy link

@vera-liu Hi Vera, it seems not working in Superset 0.13.0.

@vera-liu
Copy link
Contributor Author

turned out this was not added for reactified dashboard, https://github.com/airbnb/superset/pull/1596 should solved it.

@github35
Copy link

Thank you @vera-liu 👍

zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 17, 2021
* fix: show value on the selected series

* fix: callback
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 24, 2021
* fix: show value on the selected series

* fix: callback
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 25, 2021
* fix: show value on the selected series

* fix: callback
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 26, 2021
* fix: show value on the selected series

* fix: callback
@mistercrunch mistercrunch added the 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels label Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants