feat: Implement new Django CMS design language for Django CMS 5.1 #8320
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideThis PR implements the new Django CMS design language by introducing design tokens and theme variables, refactoring Sass to modern modules and mixins, and updating component styles, templates, and JS logic throughout the admin UI to align with the new visual system. Class diagram for updated icon mapping and new icon additionclassDiagram
class IconMap {
+getIconChar(filename)
}
class ExternalPlaceholderIcon {
+char = "E017"
}
IconMap <|-- ExternalPlaceholderIcon
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8320 +/- ##
=======================================
Coverage 89.71% 89.71%
=======================================
Files 129 129
Lines 12750 12750
=======================================
Hits 11439 11439
Misses 1311 1311 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `cms/static/cms/sass/components/_modal.scss:271-272` </location>
<code_context>
.cancel-link {
display: none;
}
+ float: inline-end;
+ &:has(.deletelink) {
+ float: inline-start;
+ }
</code_context>
<issue_to_address>
**issue:** Float logic for modal item buttons now uses inline-end and inline-start based on content.
Confirm that all target browsers support the :has() selector, or implement a fallback for those that do not.
</issue_to_address>
### Comment 2
<location> `cms/static/cms/sass/components/pagetree/_tree.scss:215-216` </location>
<code_context>
display: table-cell;
vertical-align: middle;
position: relative;
- outline: none;
- color: $white !important;
- padding-block: 8px !important;
- padding-inline: 10px !important;
</code_context>
<issue_to_address>
**issue:** Removed outline and forced color from pagetree header button.
Please ensure a visible focus indicator remains for keyboard users to maintain accessibility standards.
</issue_to_address>
### Comment 3
<location> `cms/static/cms/sass/settings/_cms.scss:16` </location>
<code_context>
+ --dca-light: #defcb9;
+ --dca-white: light-dark(#FFF, #000);
+ --dca-black: light-dark(#2a2c2e, rgb(from white calc(r - 45) calc(g - 44) calc(b - 46)));
+ --dca-primary: light-dark(var(--dca-dark), rgb(from var(--dca-light) calc(r - 45) calc(g - 44) calc(b - 46)));
+ --dca-secondary: light-dark(var(--dca-light), var(--dca-dark));
+ --dca-red: #d44848;
</code_context>
<issue_to_address>
**issue:** Primary color now uses dynamic calculation based on theme.
Please verify that the dynamic color calculation works consistently across all supported browsers, especially in those without rgb(from ...) support.
</issue_to_address>
### Comment 4
<location> `cms/tests/frontend/unit/cms.pagetree.stickyheader.test.js:84-91` </location>
<code_context>
spyOn(sticky, '_isInSideframe').and.returnValue(false);
- $('<div id="branding" style="height: 200px"></div>').prependTo(sticky.ui.container);
- expect(sticky.toolbarHeight).toEqual(null);
+ $('<div class="djangocms-admin-style"><div id="branding" style="height: 200px"></div></div>')
+ .prependTo(sticky.ui.container);
+ expect(sticky.toolbarHeight).toEqual(0);
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for when the branding element is missing.
Adding this test will verify that the fallback logic for toolbarHeight functions correctly when the branding element is not present.
```suggestion
it('saves toolbar height if in admin', function() {
spyOn(sticky, '_isInSideframe').and.returnValue(false);
$('<div class="djangocms-admin-style"><div id="branding" style="height: 200px"></div></div>')
.prependTo(sticky.ui.container);
expect(sticky.toolbarHeight).toEqual(0);
sticky._saveSizes();
expect(sticky.toolbarHeight).toEqual(200);
});
it('sets toolbarHeight to 0 when branding element is missing', function() {
spyOn(sticky, '_isInSideframe').and.returnValue(false);
// Only add the admin style container, no branding element
$('<div class="djangocms-admin-style"></div>').prependTo(sticky.ui.container);
expect(sticky.toolbarHeight).toEqual(0);
sticky._saveSizes();
// Expect toolbarHeight to remain 0 (fallback logic)
expect(sticky.toolbarHeight).toEqual(0);
});
```
</issue_to_address>
### Comment 5
<location> `cms/tests/frontend/unit/cms.pagetree.stickyheader.test.js:88` </location>
<code_context>
- expect(sticky.toolbarHeight).toEqual(null);
+ $('<div class="djangocms-admin-style"><div id="branding" style="height: 200px"></div></div>')
+ .prependTo(sticky.ui.container);
+ expect(sticky.toolbarHeight).toEqual(0);
sticky._saveSizes();
expect(sticky.toolbarHeight).toEqual(200);
</code_context>
<issue_to_address>
**nitpick (testing):** Update test description to clarify the expected initial value of toolbarHeight.
Update the test description and related comments to reflect that toolbarHeight now defaults to 0 instead of null.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+271
to
+272
| float: inline-end; | ||
| &:has(.deletelink) { |
Contributor
There was a problem hiding this comment.
issue: Float logic for modal item buttons now uses inline-end and inline-start based on content.
Confirm that all target browsers support the :has() selector, or implement a fallback for those that do not.
| --dca-light: #defcb9; | ||
| --dca-white: light-dark(#FFF, #000); | ||
| --dca-black: light-dark(#2a2c2e, rgb(from white calc(r - 45) calc(g - 44) calc(b - 46))); | ||
| --dca-primary: light-dark(var(--dca-dark), rgb(from var(--dca-light) calc(r - 45) calc(g - 44) calc(b - 46))); |
Contributor
There was a problem hiding this comment.
issue: Primary color now uses dynamic calculation based on theme.
Please verify that the dynamic color calculation works consistently across all supported browsers, especially in those without rgb(from ...) support.
| expect(sticky.toolbarHeight).toEqual(null); | ||
| $('<div class="djangocms-admin-style"><div id="branding" style="height: 200px"></div></div>') | ||
| .prependTo(sticky.ui.container); | ||
| expect(sticky.toolbarHeight).toEqual(0); |
Contributor
There was a problem hiding this comment.
nitpick (testing): Update test description to clarify the expected initial value of toolbarHeight.
Update the test description and related comments to reflect that toolbarHeight now defaults to 0 instead of null.
PeterW-LWL
reviewed
Oct 1, 2025
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements the new design language for Django CMS.
Related resources
Checklist
mainSummary by Sourcery
Implement the new Django CMS 5.1 design language by migrating to CSS variable–driven theming, updating the color palette, spacing, border radii, shadows and typography, and adjusting all core UI components to match the new spec. Also refine Sass code with modern module imports and functions, add an external-placeholder icon, and fix sticky header toolbar height logic.
New Features:
Bug Fixes:
Enhancements: