-
Notifications
You must be signed in to change notification settings - Fork 32
fix: CarouselSlidePlugin and TabItemPlugin template retrieval to respect parent template setting #314
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
Conversation
Co-authored-by: fsbraun <16904477+fsbraun@users.noreply.github.com>
|
@copilot MAke changes flake8 compliant |
Co-authored-by: fsbraun <16904477+fsbraun@users.noreply.github.com>
Fixed flake8 compliance issues:
All modified files now pass flake8 validation with the project's configuration (max-line-length=120). Commit: (hash will be in next push) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #314 +/- ##
=======================================
Coverage 89.13% 89.13%
=======================================
Files 124 124
Lines 3387 3387
Branches 288 288
=======================================
Hits 3019 3019
Misses 252 252
Partials 116 116 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fix CarouselSlidePlugin and TabItemPlugin template retrieval issue
Problem
In version 2.0.0,
CarouselSlidePluginandTabItemPluginstopped respecting their parent plugin'stemplatesetting when multiple templates are configured. This caused child plugins to always render using the first (default) template, regardless of which template was selected on the parent carousel or tab container.For example, with the following configuration:
Even when a Carousel is explicitly set to use the "fancy" template, all its slides would incorrectly render using the "default" template.
Root Cause
The issue was in the
get_render_template()methods of both plugins. They were using:This returns
instance.parentwhich is aCMSPlugininstance (the base model). TheCMSPluginmodel doesn't have atemplatefield - only the actual plugin models (likeCarouselorTab) have this field in their configuration. Without access to the template field,get_plugin_template()falls back tofirst_choice(templates), always returning the first available template.Solution
Changed the code to properly retrieve the actual plugin instance:
The
get_plugin_instance()method returns a tuple of(plugin_instance, plugin_class). Using[0]extracts the actual typed plugin instance (e.g.,Carousel) which has thetemplatefield, allowing the correct template to be used.Changes
CarouselSlidePlugin.get_render_template()indjangocms_frontend/contrib/carousel/cms_plugins.pyTabItemPlugin.get_render_template()indjangocms_frontend/contrib/tabs/cms_plugins.pytest_carousel_slide_inherits_parent_template()to verify carousel template inheritancetest_tab_item_inherits_parent_template()to verify tab template inheritanceNotes
This fix aligns with the pattern already used in the framework render mixins (
carousel/frameworks/bootstrap5.pyandtabs/frameworks/bootstrap5.py) which correctly useinstance.parent.get_plugin_instance()[0]to access the parent plugin.The change is backward compatible - if no parent exists, it falls back to using the instance itself.
Fixes #313
Original prompt
Fixes #313
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.