-
Notifications
You must be signed in to change notification settings - Fork 6
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
Remove CMFDefault interface #9
base: master
Are you sure you want to change the base?
Conversation
Hi, Cool, every help is appreciated! You can have a look how p4/p5 compatibility is implemented in other shop related packages, e.g. https://github.com/bluedynamics/bda.plone.shop Basically every ZCML registration differing between p4 and p5 is done with ZCML conditions, yes. The major compatibility issue is GenericSetup. In all other packages we register a base profile containing all common GS registration like browserlayer, catalog, rolemap and so forth, and additionally a dedicated p4 respective p5 related profile registering the delivered browser resources. Plone version dedicated templates and CSS files are postfixed with The first necessary part would be to split up GenericSetup. Further we need p5 related CSS. |
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.
Also a short change log entry would be great.
@@ -174,7 +174,6 @@ | |||
<!-- forms --> | |||
<adapter | |||
for="Products.CMFCore.interfaces.IFolderish | |||
Products.CMFDefault.interfaces.ICMFDefaultSkin |
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.
This is the add form, which is always expected as a multi adapter between context, request and optional FTI. https://github.com/plone/plone.dexterity/blob/master/plone/dexterity/browser/add.py#L38
afaict the correct fix would be two replace Products.CMFDefault.interfaces.ICMFDefaultSkin
by zope.publisher.interfaces.browser.IDefaultBrowserLayer
.
Just removal will break it.
Getting rid of Products.CMFDefault
is important, it was removed from Plone 5.1 already as core-dependency.
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.
Thanks @jensens! Will update the PR. This needs to be added to the documentation, too:
http://docs.plone.org/develop/plone/content/dexterity.html#custom-add-form-view
?
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.
Indeed, the documentation is wrong there.
Its planed to support Plone4 & Plone5. Should i add some kind of zcml:condition here?