[UI] Harden Django Showcase UI#112
Conversation
|
@greptile review |
Greptile SummaryThis PR is a comprehensive UI hardening pass across the Django Showcase: it extracts reusable CSS component classes (
Confidence Score: 5/5Safe to merge — all changes are additive UI/CSS refactoring with no data-layer or auth-boundary modifications. The PR replaces inline styles and hardcoded Tailwind utilities with named CSS component classes, extracts a reusable page-hero template, and gates external scripts behind an environment flag. The only substantive logic changes are in the ad reveal script and the Stimulus controllers, both of which the author verified at multiple viewport sizes. No server-side business logic, database schemas, or authentication paths are modified. dropdown_controller.js — the hide() override leaves aria-expanded in an inconsistent state after menu-item clicks (Turbo Drive navigation resets it, but screen readers may observe the stale value during the transition). Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Browser
participant SentryJS as sentry.js (async)
participant DropdownCtrl as dropdown_controller.js
participant AdScript as bottom-right-corner-ad.html
participant Analytics as analytics.py
Browser->>SentryJS: Page load
SentryJS->>SentryJS: readSentryConfig()
alt config.enabled and config.dsn
SentryJS->>Browser: import sentry chunk
Browser-->>SentryJS: Sentry module
SentryJS->>SentryJS: Sentry.init()
end
Browser->>DropdownCtrl: connect()
DropdownCtrl->>DropdownCtrl: super.connect()
DropdownCtrl->>Browser: addEventListener keydown closeOnEscape
DropdownCtrl->>DropdownCtrl: setExpanded(false)
Browser->>AdScript: DOMContentLoaded
AdScript->>AdScript: hasClosedCookie() adIsClosed
alt not past intro scroll
Browser->>AdScript: scroll event
AdScript->>AdScript: isPastIntro() true
AdScript->>AdScript: showAd() syncVisibleAd()
AdScript->>Browser: showContainer + rAF bw-ad--visible
end
Browser->>AdScript: close button click
AdScript->>AdScript: hideAd() setClosedCookie()
AdScript->>Browser: remove bw-ad--visible setTimeout 220ms add hidden
Browser->>Analytics: HTTP request
Analytics-->>Browser: app_environment context
alt app_environment prod
Browser->>Browser: Load plausible script
Browser->>Browser: Load webring.js
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Browser
participant SentryJS as sentry.js (async)
participant DropdownCtrl as dropdown_controller.js
participant AdScript as bottom-right-corner-ad.html
participant Analytics as analytics.py
Browser->>SentryJS: Page load
SentryJS->>SentryJS: readSentryConfig()
alt config.enabled and config.dsn
SentryJS->>Browser: import sentry chunk
Browser-->>SentryJS: Sentry module
SentryJS->>SentryJS: Sentry.init()
end
Browser->>DropdownCtrl: connect()
DropdownCtrl->>DropdownCtrl: super.connect()
DropdownCtrl->>Browser: addEventListener keydown closeOnEscape
DropdownCtrl->>DropdownCtrl: setExpanded(false)
Browser->>AdScript: DOMContentLoaded
AdScript->>AdScript: hasClosedCookie() adIsClosed
alt not past intro scroll
Browser->>AdScript: scroll event
AdScript->>AdScript: isPastIntro() true
AdScript->>AdScript: showAd() syncVisibleAd()
AdScript->>Browser: showContainer + rAF bw-ad--visible
end
Browser->>AdScript: close button click
AdScript->>AdScript: hideAd() setClosedCookie()
AdScript->>Browser: remove bw-ad--visible setTimeout 220ms add hidden
Browser->>Analytics: HTTP request
Analytics-->>Browser: app_environment context
alt app_environment prod
Browser->>Browser: Load plausible script
Browser->>Browser: Load webring.js
end
Reviews (4): Last reviewed commit: "fix: guard dropdown parent disconnect" | Re-trigger Greptile |
Greptile SummaryThis PR hardens the Built with Django frontend across 38 files: it extracts reusable CSS component classes, adds ARIA attributes and Escape-key handling to navigation dropdowns, rewrites the bottom-ad lifecycle to use CSS-class transitions with proper resize and cookie-close guards, lazy-loads Sentry via dynamic import, and gates analytics/webring behind an
Confidence Score: 4/5The changes are predominantly additive UI hardening with no database migrations or auth-boundary changes; the main risk is visual regression in production from the narrowed Tailwind safelist. All three findings are non-blocking quality issues. The narrowed Tailwind safelist is the highest-risk item — if unchanged templates contain dynamic color utility classes outside the new pattern (only -100 and -800 shades are kept), elements could silently lose styles in the production bundle. The execCommand false-return and missing super.disconnect() are confined to edge-case code paths. No auth, data, or migration changes are touched. tailwind.config.js and any template that injects Tailwind color utilities from model data — verify the new safelist covers every shade actually used at runtime. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DOMContentLoaded] --> B{hasClosedCookie?}
B -- yes --> Z[Do nothing]
B -- no --> C[maybeShowAd]
C --> D{isPastIntro scroll > 420px?}
D -- no --> E[Add scroll listener]
E --> F[scroll event]
F --> G{adClosed?}
G -- yes --> H[Remove scroll listener]
G -- no --> D
D -- yes --> I[showAd]
I --> J{adIsVisible OR adIsClosed?}
J -- yes --> Z
J -- no --> K[adIsVisible = true, Remove scroll listener, Add resize listener]
K --> L[syncVisibleAd]
L --> M{desktopMedia.matches?}
M -- yes --> N[showContainer desktop, hideContainer mobile]
M -- no --> O[showContainer mobile, hideContainer desktop]
N --> P[Remove hidden class, RAF: add bw-ad--visible]
O --> P
Q[User clicks close] --> R[hideAd]
R --> S[adIsClosed = true, Remove listeners, setClosedCookie]
S --> T[Remove bw-ad--visible from both containers]
T --> U[setTimeout 220ms]
U --> V[Add hidden to both containers]
W[resize event] --> L
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[DOMContentLoaded] --> B{hasClosedCookie?}
B -- yes --> Z[Do nothing]
B -- no --> C[maybeShowAd]
C --> D{isPastIntro scroll > 420px?}
D -- no --> E[Add scroll listener]
E --> F[scroll event]
F --> G{adClosed?}
G -- yes --> H[Remove scroll listener]
G -- no --> D
D -- yes --> I[showAd]
I --> J{adIsVisible OR adIsClosed?}
J -- yes --> Z
J -- no --> K[adIsVisible = true, Remove scroll listener, Add resize listener]
K --> L[syncVisibleAd]
L --> M{desktopMedia.matches?}
M -- yes --> N[showContainer desktop, hideContainer mobile]
M -- no --> O[showContainer mobile, hideContainer desktop]
N --> P[Remove hidden class, RAF: add bw-ad--visible]
O --> P
Q[User clicks close] --> R[hideAd]
R --> S[adIsClosed = true, Remove listeners, setClosedCookie]
S --> T[Remove bw-ad--visible from both containers]
T --> U[setTimeout 220ms]
U --> V[Add hidden to both containers]
W[resize event] --> L
Reviews (2): Last reviewed commit: "feat: harden Django showcase UI" | Re-trigger Greptile |
|
@greptile review |
|
@greptile review |
Summary
Verification