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

pug_py widget #113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

pug_py widget #113

wants to merge 1 commit into from

Conversation

jperon
Copy link
Contributor

@jperon jperon commented Nov 21, 2023

This is a widget adapted from https://github.com/berhalak/my-widgets/tree/main/fiddle, to directly develop custom widgets with pug and Python (thanks to Brython).

Do you think it would fit within this repo? There is an example here.

Copy link

netlify bot commented Nov 21, 2023

Deploy Preview for boisterous-sunburst-a5c941 ready!

Name Link
🔨 Latest commit a48f666
🔍 Latest deploy log https://app.netlify.com/sites/boisterous-sunburst-a5c941/deploys/65cf9427fbedf30008cfa888
😎 Deploy Preview https://deploy-preview-113--boisterous-sunburst-a5c941.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jperon
Copy link
Contributor Author

jperon commented Nov 22, 2023

Here is an example query to test the widget in the sample linked previously:

select
   t.N x_Period,
   avg(g.Grade) filter (where c.Brief = "H" and p.Full_Name = "$Person") y_H_PUPIL,
   avg(m.Avg) filter (where c.Brief = "H") y_H_CLASS,
   avg(g.Grade) filter (where c.Brief = "E" and p.Full_Name = "$Person") y_E_PUPIL,
   avg(m.Avg) filter (where c.Brief = "E") y_E_CLASS
 from Grades g
 join Grades_summary_category_period m on g.Period = m.Period and g.Category = m.Category
 join Persons p on g.Person = p.id
 join Periods t on g.Period = t.id
 join Categories2 c on g.Category = c.id
 group by x_Period

@berhalak berhalak self-assigned this Dec 4, 2023
@berhalak
Copy link
Contributor

berhalak commented Dec 7, 2023

Here is an example query to test the widget in the sample linked previously:

select
   t.N x_Period,
   avg(g.Grade) filter (where c.Brief = "H" and p.Full_Name = "$Person") y_H_PUPIL,
   avg(m.Avg) filter (where c.Brief = "H") y_H_CLASS,
   avg(g.Grade) filter (where c.Brief = "E" and p.Full_Name = "$Person") y_E_PUPIL,
   avg(m.Avg) filter (where c.Brief = "E") y_E_CLASS
 from Grades g
 join Grades_summary_category_period m on g.Period = m.Period and g.Category = m.Category
 join Persons p on g.Person = p.id
 join Periods t on g.Period = t.id
 join Categories2 c on g.Category = c.id
 group by x_Period

Yes definitely, it looks great. We can store the source code here. About publishing it to getgrist.com, I'd need to ask the Product team, but source code of this widget definitely can be stored and hosted in this repository for others to see.
Regarding submodule, we are currently in the middle of reorganizing the way we build custom widgets, @alexmojaki is brainstorming ideas and one of the ideas is actually using submodules. I'll add him to this PR as he might find it useful.

@jperon jperon marked this pull request as ready for review December 23, 2023 09:53
@paulfitz
Copy link
Member

paulfitz commented Jan 5, 2024

Hi @jperon, sorry for the delay responding to you. Thanks for preparing this. @alexmojaki tried it out and reported that it doesn't quite work as is since the code contains <script src="/grist-plugin-api.js">.

The larger hold-up is that we're not sure how to present this widget to users. It would be easier if the custom widget drop-down in the Grist app had information about authorship and maybe something about degree of expertise required to use. We've designed something like that in the past, but haven't had a chance to build it yet.

@jperon
Copy link
Contributor Author

jperon commented Jan 5, 2024

Thank you very much for the attention you pay to this!

@alexmojaki tried it out and reported that it doesn't quite work as is since the code contains <script src="/grist-plugin-api.js">.

Just fixed it; it had been a workaround for a bug in a past release.

The larger hold-up is that we're not sure how to present this widget to users. It would be easier if the custom widget drop-down in the Grist app had information about authorship and maybe something about degree of expertise required to use. We've designed something like that in the past, but haven't had a chance to build it yet.

I don’t really have a strong idea about it. The level of expertise could be quite low to begin (for very simple things like the iframe here, but will grow quite fast for more complicated ones. Tell me whether you’d like me to change anything; otherwise, I could add you (or other ones) as owner of this repo, in order to ease changes.

@jperon
Copy link
Contributor Author

jperon commented Jan 7, 2024

Thinking a bit more about <script src="/grist-plugin-api.js">: IMHO, the main interest about such a widget would be the ability to self-host it, or event to integrate it with grist-electron. But having an absolute url to docs.getgrist.com/grist-plugin-api.js would make it internet-dependent. As far as I can see, a self-hosted grist-core does serve /grist-plugin-api.js; so why wouldn’t it be possible?

@paulfitz
Copy link
Member

paulfitz commented Jan 8, 2024

Thinking a bit more about <script src="/grist-plugin-api.js">: IMHO, the main interest about such a widget would be the ability to self-host it, or event to integrate it with grist-electron. But having an absolute url to docs.getgrist.com/grist-plugin-api.js would make it internet-dependent. As far as I can see, a self-hosted grist-core does serve /grist-plugin-api.js; so why wouldn’t it be possible?

There is in fact a mechanism now for serving custom widgets in place in grist-core and grist-electron https://github.com/gristlabs/grist-core/blob/a59132108fb1d831cbf99b767c55efbd31fceb2f/app/server/lib/FlexServer.ts#L641-L648; in that case any reference to grist-plugin-api.js is served locally. Configuration remains a bit too complicated though.

The main problem with merging with <script src="/grist-plugin-api.js"> is that it would end up pointing to https://gristlabs.github.io/grist-plugin-api.js which does not exist.

@jperon
Copy link
Contributor Author

jperon commented Feb 14, 2024

Are there things I’d have to do on this PR? No hurry, just to know whether something is missing.

@paulfitz
Copy link
Member

Are there things I’d have to do on this PR? No hurry, just to know whether something is missing.

The files changed are looking a bit strange now, perhaps a rebase is needed?

What did you think of this issue?

The main problem with merging with <script src="/grist-plugin-api.js"> is that it would end up pointing to https://gristlabs.github.io/grist-plugin-api.js which does not exist.

I think we do need someone to add a way in grist-core to show who controls a widget in the list of widgets offered to users. Up until now, it has been Grist Labs, like grist-core itself, but once this lands one of the widgets will be controlled by someone else, and I think we need to communicate that somehow.

I am interested in getting this landed. And also widgets built on it, like your Kanban widget. I was wondering how that would work exactly, though, would the code need to be pulled out from where it is stored in Grist configuration?

@jperon
Copy link
Contributor Author

jperon commented Feb 15, 2024

The files changed are looking a bit strange now, perhaps a rebase is needed?

Done.

What did you think of this issue?

It’s already fixed. I had a question afterwards, but the link is correct now.

I think we do need someone to add a way in grist-core to show who controls a widget in the list of widgets offered to users. Up until now, it has been Grist Labs, like grist-core itself, but once this lands one of the widgets will be controlled by someone else, and I think we need to communicate that somehow.

That said, I’d see no problem (and even prefer) to hand control to Grist team if you don’t have objections. I could even do a copyright assignment to Grist team, as my work is essentially surface tuning, but the whole merit goes to @berhalak (with his fiddle widget).

I am interested in getting this landed. And also widgets built on it, like your Kanban widget. I was wondering how that would work exactly, though, would the code need to be pulled out from where it is stored in Grist configuration?

Good point: for now, it’s essentially copy/paste from example widgets. It’s the reverse side of the medal: the good point is that the code is hosted within the document itself (so doesn’t depend on a web connection as soon as the instance hosts the widget). IMHO, the best way to share widgets based on this one (like Kanban, Signature or Tree) would be to have a repository for examples (that could serve as documentation too).

@paulfitz
Copy link
Member

That said, I’d see no problem (and even prefer) to hand control to Grist team if you don’t have objections. I could even do a copyright assignment to Grist team, as my work is essentially surface tuning, but the whole merit goes to @berhalak (with his fiddle widget).

That's an interesting option. You're being a bit humble with the "surface tuning" :). I'd prefer to keep you on maintaining this if possible, we are over-stretched. How about this option: we fork or move the repository to the gristlabs organization, unchanged, that's the one we list, and you make changes to it via pull requests? That way there's one extra level of protection for end users if you are hacked, for example.

I am interested in getting this landed. And also widgets built on it, like your Kanban widget. I was wondering how that would work exactly, though, would the code need to be pulled out from where it is stored in Grist configuration?

Good point: for now, it’s essentially copy/paste from example widgets. It’s the reverse side of the medal: the good point is that the code is hosted within the document itself (so doesn’t depend on a web connection as soon as the instance hosts the widget). IMHO, the best way to share widgets based on this one (like Kanban, Signature or Tree) would be to have a repository for examples (that could serve as documentation too).

The problem as I see it is that there are many more people who could use a Kanban widget (picked from a dropdown) than could edit a custom widget's configuration. Imagine if using the Calendar widget required pasting in some html/python boilerplate. That would dramatically reduce who could use it in practice.

Would there be a way to bundle up most of pug_py, so a new custom widget could include that bundle and just the extra code needed to initialize it?

@jperon
Copy link
Contributor Author

jperon commented Feb 16, 2024

I'd prefer to keep you on maintaining this if possible, we are over-stretched. How about this option: we fork or move the repository to the gristlabs organization, unchanged, that's the one we list, and you make changes to it via pull requests? That way there's one extra level of protection for end users if you are hacked, for example.

That would be the best option IMHO.

The problem as I see it is that there are many more people who could use a Kanban widget (picked from a dropdown) than could edit a custom widget's configuration. Imagine if using the Calendar widget required pasting in some html/python boilerplate. That would dramatically reduce who could use it in practice.

Would there be a way to bundle up most of pug_py, so a new custom widget could include that bundle and just the extra code needed to initialize it?

That’s a good idea: I suggest we keep this one as a good tool for POC; I’d then put together a boilerplate in which the code could be copied in order to bundle a new full-blown widget.

@jperon
Copy link
Contributor Author

jperon commented Feb 16, 2024

There is even an option to transfer ownership of a repository; but for that, I’d need (temporarily!) write access on gristlabs organization.

@jperon
Copy link
Contributor Author

jperon commented Feb 17, 2024

I found a better way: transferring to @paulfitz. Then you’ll be able to transfer it to gristlabs.

@paulfitz
Copy link
Member

I found a better way: transferring to @paulfitz. Then you’ll be able to transfer it to gristlabs.

Transferred! Thanks @jperon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants