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

Add RenderSnippet to Renderer to facilitate their embedding within other templates #410

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

Conversation

sdqri
Copy link

@sdqri sdqri commented Apr 3, 2024

Description

Recently I was using go-echarts in one of my projects and I would have liked it if It was possible to embed my charts into custom templates, but I couldn't find a straightforward way to achieve this. I found this article that talks about the same problem with go-echarts and solves it by implementing a custom renderer. I find it strange that there isn't a built-in way to do this, considering that embedding charts instead of firing a standalone HTTP page or using iframes is expected by most users in production.
I have a simple implementation to do this by adding a RenderSnippet function to the Renderer interface, which is a charm when working only with charts. But the only problem is that both pageRender & chartRender use the same Renderer interface and defining the correct behavior for this method in PageRender can be a little tricky. I've solved it by collecting all elements & scripts in singular fields.

Type of change

  • Bug fix (Non-breaking change which fixes an issue)
  • New feature (Non-breaking change which adds functionality)
  • Breaking change (Would cause existing functionality to not work as expected)
  • Docs
  • Others

Example

package main

import (
	"math/rand"
	"os"
	"text/template"

	"github.com/go-echarts/go-echarts/v2/charts"
	"github.com/go-echarts/go-echarts/v2/opts"
)

// generate random data for bar chart
func generateBarItems() []opts.BarData {
	items := make([]opts.BarData, 0)
	for i := 0; i < 7; i++ {
		items = append(items, opts.BarData{Value: rand.Intn(300)})
	}
	return items
}

func main() {
	// create a new bar instance
	bar := charts.NewBar()
	// set some global options like Title/Legend/ToolTip or anything else
	bar.SetGlobalOptions(charts.WithTitleOpts(opts.Title{
		Title:    "My first bar chart generated by go-echarts",
		Subtitle: "It's extremely easy to use, right?",
	}))

	// Put data into instance
	bar.SetXAxis([]string{"Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"}).
		AddSeries("Category A", generateBarItems()).
		AddSeries("Category B", generateBarItems())

	// Render chartSnippet that has Element & Script fields
	chartSnippet := bar.RenderSnippet()

	// Embedd element & script into a cusotm template
	tmpl := "{{.Element}}{{.Script}}"
	t := template.New("snippet")
	t, err := t.Parse(tmpl)
	if err != nil {
		panic(err)
	}

	err = t.Execute(os.Stdout, chartSnippet)
	if err != nil {
		panic(err)
	}
}

@Koooooo-7 Koooooo-7 added the attention Attention may include breaking changes label Apr 7, 2024
@Koooooo-7
Copy link
Member

HI @sdqri thx for ur great idea and contributions!
It seems a feasible way to do the extract of charts rendered JS snippets.
I will check whether it has the potential issues and fix the CI asap.

@Koooooo-7 Koooooo-7 self-requested a review April 7, 2024 11:50
@Koooooo-7 Koooooo-7 added the enhancement New feature or request label Apr 7, 2024
@neomantra
Copy link
Collaborator

I'm collaborating on this gonb issue -- the goal is to have Echarts within Golang Jupyter notebooks, both in browser and VSCode. I think this feature will be helpful for that effort.

@Koooooo-7
Copy link
Member

Hi @sdqri , I checked this PR and ran the sample you provided. It works fine to output escape snippets (maybe we need a config that enable escape content or not? ).

When I try to render in page, the Elements and Scripts are grouped by the templates, it seems weird. I suppose this functions is only needed in single charts, so the page part can be ignored.
If user does need the batch charts render to extract snippets, it could be done simply in an for range with charts.

TBH, I hasn't decide that where to put this feature in.
The Default Render in go-echarts amis to get a fully final product (HTML/stream).
About the Extract Options, Extract Snippet... which wants to get the raw part of partial render results are more like the new extension/function stuff to me, they are not in the render scope.

Currently, I'm thinking about the two options:

  • Include a new Extractor interface (TBD) instead of the Render which only support single chart.
  • Put the Snippet Render as an extension out of go-echarts such as snapshot-chromedp (extra effort for templates maintain)

Any thoughts on this plz let me know :).

@janpfeifer
Copy link

hi,

In the meantime I created a small library to integrate go-echarts into gonb, by parsing the generated HTML page, and extracting the needed javascript code, and re-rendering it in the Jupyter notebook.

It's working great 😃 See examples page, it is the HTML export of a working Jupyter notebook.

Btw, would it be ok to add an entry in the Features section of the README.md file with a line like: "Available also in Go Jupyter Notebooks with GoNB, see examples." ?

@sdqri
Copy link
Author

sdqri commented Apr 20, 2024

Apologies for the delay in responding and thanks for the review. I totally understand your concern regarding the integration of this feature into the Render interface. I must admit that, since I'm not entirely familiar with the project's structure and scope, I may lack sufficient knowledge to decide which of these options are better for project. Personally, I opted to use a custom renderer in my use case. However, since I believed this feature would be beneficial to a significant portion of the library's users, my attempt in the pull request was to implement it as simply as possible, with minimal changes to the project structure.

Unfortunately, since the renderer abstracts both component and page into one bucket, any implementation that wants to change this will lead to some unintended overloading of pageRender, with a potentially redundant method, and it also will break backward compatibility of the Render interface.

Nevertheless, I still strongly support the integration of such a feature directly into the library, and I'll be happy to contribute to make this possible.

@Koooooo-7
Copy link
Member

Btw, would it be ok to add an entry in the Features section of the README.md file with a line like: "Available also in Go Jupyter Notebooks with GoNB, see examples." ?

That's a great job! and of course, for now I will add it as the Ecosystem section in go-echarts, and try to do a better category later.

@Koooooo-7
Copy link
Member

Hi @sdqri , thx for your thoughts. and I agree what you mentioned is reasonable.

I will do the rest of things (sorry that may not be that quick since I don't have much time on it recently):

  1. Check whether there is a way to distinguish the page and chart renders better (with min changes), and then, we can provide more generic api for specific type (Page/Charts) for now and future.

  2. May create a new Extractor API which use the same workable templates you changed in go-echarts Render and Extractor. The new api is only for extracting the things part of the whole HTML/Charts configurations. ( we provide the functions and the sharing templates, so we don't need to duplicate the templates and codes, as well as the potentially redundant stuff)

@Koooooo-7 Koooooo-7 mentioned this pull request Apr 20, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention Attention may include breaking changes enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants