-
-
Notifications
You must be signed in to change notification settings - Fork 753
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
Listing Block template for Page/News Item/Event based on content type. #6055
Conversation
✅ Deploy Preview for plone-components canceled.
|
config?.getComponent && | ||
config.getComponent({ name: 'ListingTemplate', dependencies: [type] }) | ||
.component) || | ||
contentTemplate[type]; |
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.
@iFlameing Using the registry here is good, but I think the way RenderAccordingToContentType is currently implemented, it needs to know too much about the specific listing variations that exist (i.e. the boolean props for image and grid).
Also the contentTemplate mapping should not be hardcoded here. That is what the registry lookup is for.
Instead I would suggest to get rid of RenderAccordingToContentType, and do the registry lookup directly in the variation templates (i.e. in DefaultTemplate, SummaryTemplate, and Grid).
Each variation should use a different component name in the registry lookup:
- default variation -> ListingTemplate component type
- summary variation -> ListingWithImageTemplate component type
- grid variation -> ListingGridTemplate component type
If you have questions about how to use the registry while I am asleep, @sneridagh can probably help.
@iFlameing For migration / backwards compatibility, we have 2 options:
I like option 2 better. It's a lot simpler for users, and still pretty easy to implement. We'll need a section in the upgrade guide to explain how to configure this mapping if the user wants something other than the default one. |
@davisagli pleaser review it once again. I will rename the variable once it is done. Same for default content type. I will register it wit config.register component. |
@iFlameing this PLIP/PR is mentioned in the Plone roadmap that is about to being published next week. We should try to put some effort into this. Maybe some folks at the Salamina sprints can help. cc @davisagli @sneridagh @fredvd @jackahl |
We are trying a different solution in Volto Light Theme: kitconcept/volto-light-theme#462 -- that one introduces a Summary component which makes it easier to add a new content type's summary that is used in several different blocks, and also avoids the need to migrate existing block variation names. If it works well then we can consider handling it that way in Volto or Seven in the future. |
Still long road to go. Fixes #4765
Replicate same things for search.
📚 Documentation preview 📚: https://volto--6055.org.readthedocs.build/