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

Expose patternslib #1191

Closed
wants to merge 2 commits into from
Closed

Expose patternslib #1191

wants to merge 2 commits into from

Conversation

jcbrand
Copy link
Member

@jcbrand jcbrand commented Oct 17, 2015

Add the patternslib patterns to version control so that they can eventually be exposed via the registry.

@MrTango is working on merging pat-toggle @timitos is working on autofocus and I'm working on sortable.

@vangheem
Copy link
Member

@jcbrand if you're trying to replace the mockup sortable, just be careful, it's used in quite a few other patterns.

This is a lot to include and not actually use. I'm not sure how I feel about shipping with something like this. It seems like it would commit Plone to always supporting those packages. I think I prefer people who want to use the additional patterns, to include it themselves. Or we create a collective.patternslib package?

@jcbrand
Copy link
Member Author

jcbrand commented Oct 18, 2015

About sortable, I don't think I'll get around to it this sprint still.

About exposing all patterns, the main advantage I can see is that people can easily add and use patternslib patterns in their own Plone sites or addons by simpling adding registry entries.

People regularly ask me how they can use core Patternslib patterns, and being able to just add a registry entry is a nice simple solution.

I'm not sure collective.patternslib makes sense if some patternslib patterns will be in core Plone.

We can only selectively expose those patterns being actually used (two candidates for now are toggle and autofocus), but then we lose the ability to easily add other patternslib patterns.

@vangheem
Copy link
Member

I need other people opinions on this I guess. I'm also worried about adding all this on a point release.

@jcbrand
Copy link
Member Author

jcbrand commented Oct 19, 2015

Sure, perfectly reasonable.

Request for comments: @thet, @frapell, @fulv, @MrTango, @timitos, @pbauer, @pilz, @neaj, @ebrehault

@jcbrand
Copy link
Member Author

jcbrand commented Oct 20, 2015

@vangheem I think this is a good example why we need to make it easy to expose Patternslib patterns: https://community.plone.org/t/collapsiblesections-js-is-gone-long-live-pat-collapsible/1047

There are good reasons to start using existing Patternslib patterns, but there is still a lot of confusion. I'd like to be able to tell people to just add registry entry for the pattern they want to use.

@thet
Copy link
Member

thet commented Oct 20, 2015

I'm a bit concerned about library size. The whole Patternslib library has 6 MB. Including all dependencies it's 42MB. There are a lot of .gitignore statements, which excludes images, but there are no for other dependencies.

At ploneconf we agreed that we want to merge duplicate patterns into those in Patternslib and move general useable patterns from mockup to Patternslib.
If we all agree on that, we should start shipping Patternslib.

What about moving the whole bower components directory out into a seperate plone core package? That would allow us to update bower dependencies independently from the Plone release cycles and Keep CMFPlone smaller in size.

@jcbrand
Copy link
Member Author

jcbrand commented Oct 20, 2015

What about moving the whole bower components directory out into a seperate plone core package? That would allow us to update bower dependencies independently from the Plone release cycles and Keep CMFPlone smaller in size.

That's kind of what @vangheem already suggested, but he mentioned collective.*. I think I'm fine with this, but IMO it needs to be plone.* and if not now already, eventually it'll need to be a dependency of Products.CMFPlone.

@jcbrand
Copy link
Member Author

jcbrand commented Oct 20, 2015

At ploneconf we agreed, that we want to merge duplicate patterns into those in Patternslib and move general useable patterns from mockup to Patternslib.
If we all agree on that, we should start shipping Patternslib.

I don't think that's terribly controversial. @vangheem and I discussed and agreed on this at PLOG already. However, how to properly do this (with minimal disruption) is a challenge :)

@thet
Copy link
Member

thet commented Oct 20, 2015

agree, agree.
and sorry in advance, if I rephrase existing ideas.

@jcbrand
Copy link
Member Author

jcbrand commented Oct 20, 2015

No need to apologize @thet! :)

@pilz
Copy link
Member

pilz commented Dec 2, 2015

We have now created a package plone.patternslib which does include a subset of the Patternslib patterns into Plone 5 and it seems to work without major surprises. The package is here https://github.com/syslabcom/plone.patternslib and I am happy to move it to plone or somewhere else. @vangheem @thet

@jensens
Copy link
Sponsor Member

jensens commented Dec 6, 2015

package was moved meanwhile and is here https://github.com/plone/plone.patternslib

@thet
Copy link
Member

thet commented Dec 15, 2015

I think this issue can be closed, because the problem is solved with https://github.com/plone/plone.patternslib
To the authors, @pilz @jcbrand : Do we need any further action, like making sure plone.patternslib is going to be included in core? Or should it better live as an extra addon?

Reopen this pull-request, if needed.

@thet thet closed this Dec 15, 2015
@jcbrand
Copy link
Member Author

jcbrand commented Dec 15, 2015

making sure plone.patternslib is going to be included in core?

For that to happen, core could would have to start relying on (at least some) Patternslib patterns. Since there is overlap between the functionality provided by Mockup patterns and Patternslib patterns, this could be a desirable outcome in the medium term, by deprecating certain Mockup patterns in favor of Patternslib ones.

@jensens jensens deleted the expose-patternslib branch January 3, 2016 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants