-
Notifications
You must be signed in to change notification settings - Fork 5
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
[RFC] Show provided widgets #38
Comments
Concept: ✔️ Interested to see what others think though. |
The second syntax would offer potential for future additions without breaking existing code. I'd go with that one. |
Ok, because some people are against the idea. Let's just make a little poll. Just add the 👍 or 👎 as reaction to this answer. |
is it possible to read it from theme.yml? that might also be a place to look for it
|
I agree with @jadwigo. The main goal of composer is to manage dependencies and not to provide information about theming. In the case you have many widgets, the I will not necessarely agree for However, like @PhillippOhlandt said, this solution involve much work. |
If we decide to use another file. All other things like icon and screenshots should be moved too. Otherwise its not very consistent. But that would be a big break. Maybe we keep everything in composer.json for now and move to another file with Bolt 4? |
Actually… TBH… I think I like the idea. Maybe going with
Nah, we just deprecate the old behaviour and read from both sources. |
I like the idea of |
Ok, after a small talk with @GawainLynch we think it's a good idea to have a
It will be the new place for the icon, the screenshots and of course, our widgets. I will implement it in a way that the values inside the That way we still support the definition in the |
I think it is not relevent to hide the file (the dot). Hidding file/dir like that is more for autogenerated stuff, ignoring mechanisms (.gitignore, .npmignore) or IDE specific files. I think it will be better without the dot. |
@WedgeSama: But @GawainLynch likes dot files :) |
Yeah, I totally disagree… there are dots on the box, damn it 😜 |
And we now have the |
I'm personally in favour of leaving the essentials as they are now in composer.json but since templates are already defined in theme.yml then it makes sense to me to define widgets in the same place too, I don't see any value in adding yet another metadata file since that is what theme.yml already does. The reason icon and screenshots are in composer.json are because they are needed for the extensions store, although in principle we could just have them configured as fields on the submit form rather than needing to be in the composer.json, at the time it seemed like it may be useful as we may want to show screenshots on the |
@rossriley: If we move it outside of composer.json, I would do it for themes and extensions. Not in theme.yml for themes and something different for extensions. Thing is, we don't know what stuff will follow in the future. And people convinced me that defining everything in composer.json would bloat the file. Having an extra file makes sense then. |
And for the extra fields on the submit form: That's a bad idea. When people move files around or delete them in the repo and forget to update the store entry, the package detail page would be broken, etc. |
Well, variables in theme.yml are usually used in the template or by bolt, which isn't really metadata, right? It's actual data, since it's used by the system (like the templatefields definitions for example). This would just be for extensions.bolt.cm and similar systems to read, not for actual use by the theme or bolt once it's installed (unless I've misunderstood something). |
Hey,
now where we can include widget locations in themes, I would find it very useful when the extension store could list them.
My idea is to add that to the
extra
section in the composer.json. Something likeor with a description
Personally, I prefer the one with the description, because then it's easier for the user and can act like a little documentation. Also, more metadata is always better :)
I would take care of the implementation in the extension store then.
The text was updated successfully, but these errors were encountered: