-
Notifications
You must be signed in to change notification settings - Fork 547
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
feat/page-opts: add option to override page opts #167
Conversation
opt(p) | ||
} | ||
return p | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AndreZiviani Sorry to interfere like this, i forked it and i see you made the same changes. You can add Layout options ass well
func WithLayoutOpts(l Layout) PageOpts {
return func(p *Page) {
p.Layout = l
}
}
@@ -30,6 +30,22 @@ type Page struct { | |||
Layout Layout | |||
} | |||
|
|||
type PageOpts func(p *Page) | |||
|
|||
func (p *Page) SetPageOptions(opts ...PageOpts) *Page { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AndreZiviani Also i think you can add the opts
in the Page's constructor it wont break anything
closed via #357 |
I was trying to use the upstream lib (https://go-echarts.github.io/go-echarts-assets/assets/echarts.min.js) instead of the forked (https://go-echarts.github.io/go-echarts-assets/assets/echarts.min.js) using the
AssetsHost
option inInitialization
struct.The problem is that
page := components.NewPage()
creates a page with default options (and do not have an option to override them), so when I added a chart usingpage.AddCharts(my_chart)
theAssetsHost
option configured in the chart is ignored. Since we can have multiple graphs per page I thought that the best way to fix this is to implement an options system like the one used in charts.I think only the option
AssetsHost
is usefull in the page context, maybe create a new struct with only this option?If anyone have a better idea please leave a comment