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

simple csv download functionality #4303

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

simple csv download functionality #4303

wants to merge 2 commits into from

Conversation

louispt1
Copy link
Contributor

This is not fully ready for merging. I'm making a pull request because I'm a little stuck on how to hide/show the download csv button depending on the chart conditions. I know there is a 'can_be_shown_as_table' variable in charts, but it's always undefined by the time the table is being rendered... quite sure I'm missing something obvious here 🙃.

I could implement a solution based on the type of chart being rendered, but it feels like the long way around. Maybe you know how this works @noracato?

Everything else is working quite well - this solution is only for charts that can be viewed as tables currently - you can download the other chart data but it's never really pretty.

P.s. - This method (from base_chart_view.coffee) looks like it's for updating the icons depending on conditions? Played around with this but I couldn't even get it to trigger with console.logs, nor find the download_csv element.

update_header: =>
    # Skip re-rendering if this chart is about to be replaced.
    return if @model.get('will_replace')

    id = @model.get 'chart_id'
    @$el.data('chart_id', id)
    @$el.find('h3').html(@model.get("name"))
    @$el.attr('data-block_ui_on_refresh', @block_ui_on_refresh())
    @$el.find('a.chart_info').toggle(@model.get('has_description'))
    @$el.find(".actions a.chart_info").attr "href", "/descriptions/charts/#{id}"
    @$el.find(".actions a.zoom_chart").attr "href", "/output_elements/#{id}/zoom"

    if @canDownloadImage()
      # saveAsPNG is exposed by app/javascripts/packs/app.ts
      #
      # Unbind any previously assigned saveAs event to prevent it being
      # triggered multiple times.
      @$el.find(".actions a.chart_to_image")
        .show()
        .off('click')
        .on('click', (event) -> BaseChartView.saveAsPNG(event, App.scenario.get('id')))
    else
      @$el.find(".actions a.chart_to_image").hide()

    @$el.find('.actions a').removeClass('loading')

    @format_wrapper = if @$el.parents(".fancybox-inner").length > 0
      @$el.parents(".fancybox-inner")
    else
      @$el

    @format_wrapper.find("a.chart_format, a.table_format").hide()
    if @model.can_be_shown_as_table()
      if @model.get 'as_table'
        @format_wrapper.find("a.chart_format").show()
      else
        @format_wrapper.find("a.table_format").show()

    @$el.find(".chart_not_finished").toggle @model.get("under_construction")
    @$el.find("a.default_chart").toggle @model.wants_default_button()
    @$el.find('a.show_related').toggle @model.wants_related_button()
    @$el.find('a.show_previous').toggle @model.wants_previous_button()

    @update_lock_icon()
    @$el.find('.actions').show()

    this

@louispt1 louispt1 linked an issue Jul 19, 2024 that may be closed by this pull request
@louispt1 louispt1 requested a review from noracato July 19, 2024 07:28
@louispt1
Copy link
Contributor Author

Because 'can_be_shown_as_a_table' is set to true in this class, and then overrided by other derived classes, the condition is always true at this point. So it always shows either the table or chart icon ... except in practice sometimes it doesn't show either.

image

I'm also a tad confused about whether both the app/javascript and app/assets/javascripts are in play - certainly the assets js is used, but is the other js still used in the etmodel?

Copy link

This pull request has had no activity for 60 days and will be closed in 7 days. Removing the "Stale" label or posting a comment will prevent it from being closed automatically. You can also add the "Pinned" label to ensure it isn't marked as stale in the future.

@github-actions github-actions bot added the Stale Issue had no activity for 60 days and will be, or has been, closed. label Sep 21, 2024
@louispt1 louispt1 added Pinned Will never be marked as stale or auto-closed. and removed Stale Issue had no activity for 60 days and will be, or has been, closed. labels Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pinned Will never be marked as stale or auto-closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The data behind all charts should be possible to download as CSV
1 participant