feat(defcon): Meshtastic × DEF CON 34 logo + accessible event theme#376
Conversation
Give the DEF CON event mode a dedicated branded header and fix low-contrast theming on dark mode. Logo - Add the full Meshtastic × DEF CON 34 crossover mark (assets/img/defcon.svg, svgo-optimised, transparent so it adapts to light/dark) and show it in LogoHeader as "M × logo", matching the Open Sauce treatment. Accessible theming - Buttons and step badges use a solid --accent instead of an accent gradient, with an auto-contrast --on-accent text colour (black/white chosen by WCAG luminance). - applyEventTheme now drives --accent from the theme's dedicated accent colour when set, falling back to primary. DEF CON's primary (#0D294A navy) is a dark brand colour that was illegible as an accent on a dark UI; its accent (#E0004E) is used instead. Other events are unchanged. - --primary-color tracks the accent so the .text-meshtastic / .bg-meshtastic / .border-meshtastic brand utilities (link/plug icons, brand buttons) stay legible. - Extract resolveEventAccentVars() (pure) and add unit tests for the accent-selection and on-accent contrast logic.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR derives contrast-aware accent CSS variables, applies solid accent styling to buttons and badges, and adds a responsive DEF CON branding variant to ChangesAccent theming
DEF CON header branding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant applyEventTheme
participant resolveEventAccentVars
participant documentElement
participant AccentUI
applyEventTheme->>resolveEventAccentVars: theme colors
resolveEventAccentVars-->>applyEventTheme: accent CSS variable map
applyEventTheme->>documentElement: set CSS variables
documentElement-->>AccentUI: accent background and foreground variables
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
utils/eventManifest.ts (1)
166-210: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the misplaced JSDoc comment to
applyEventTheme.The first JSDoc block (lines 166-170) describes applying the theme to the UI ("overriding the accent CSS variables", "all the wiring branding needs"), but it is currently stacked above the documentation for
resolveEventAccentVars(a pure function). Consider moving it down to sit immediately aboveapplyEventTheme.♻️ Proposed refactor
Remove the misplaced comment from
resolveEventAccentVars:-/** - * Re-tint the UI from the edition theme by overriding the accent CSS variables - * defined in assets/css/main.css. The whole interface (logo glow, gradient - * title, buttons) reads these, so this is all the wiring branding needs. - */ /** * Resolve the accent CSS variables an event theme drives. Pure (no DOM) so the * accent-selection and on-accent contrast logic is unit-testable. Returns nullAnd move it to immediately precede
applyEventTheme:+/** + * Re-tint the UI from the edition theme by overriding the accent CSS variables + * defined in assets/css/main.css. The whole interface (logo glow, gradient + * title, buttons) reads these, so this is all the wiring branding needs. + */ export function applyEventTheme(theme?: EventFirmwareTheme | null): void {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/eventManifest.ts` around lines 166 - 210, Move the first JSDoc block describing UI theme application so it immediately precedes applyEventTheme. Keep the resolveEventAccentVars JSDoc directly above resolveEventAccentVars, with no unrelated changes.components/LogoHeader.vue (2)
337-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Tailwind utility classes for responsive sizing.
The custom CSS for
.logo-icon-defconcan be replaced with standard Tailwind utility classes (h-24,sm:h-28,md:h-32). As per coding guidelines, use Tailwind utility classes for styling to avoid adding unnecessary custom CSS.♻️ Proposed refactor
Update the template (around line 126) to use the Tailwind classes. You can also move the helpful explanatory comment to the HTML template above the image:
<!-- The full DEF CON 34 mark (mesh burst + skull) is a detailed square, so it runs a little larger than the M to let the artwork read. It ships transparent and theme-adaptive: the opaque gradient mesh reads on either background and the skull's knockout face shows the page behind it, so it needs no filter/tile. --> <img src="`@/assets/img/defcon.svg`" class="logo-icon-event h-24 sm:h-28 md:h-32" :alt="$t('defcon_logo')" >Then, remove the custom CSS block entirely:
-/* The full DEF CON 34 mark (mesh burst + skull) is a detailed square, so it runs - a little larger than the M to let the artwork read. It ships transparent and - theme-adaptive: the opaque gradient mesh reads on either background and the - skull's knockout face shows the page behind it, so it needs no filter/tile. */ -.logo-icon-defcon { - height: 6rem; -} - -@media (min-width: 640px) { - .logo-icon-defcon { - height: 7rem; - } -} - -@media (min-width: 768px) { - .logo-icon-defcon { - height: 8rem; - } -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/LogoHeader.vue` around lines 337 - 355, Replace the responsive height rules for .logo-icon-defcon with Tailwind classes h-24 sm:h-28 md:h-32 on the DEF CON image in the template. Move the existing explanatory comment above that image if desired, then remove the .logo-icon-defcon custom CSS block while preserving the image’s other classes and attributes.Source: Coding guidelines
104-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider deduplicating the logo header layout.
This new event branch continues a pattern of duplicating the entire structural wrapper—including the Meshtastic logo glow, the
×separator, the title gradient, and the event fallback bindings.While following the existing style is fine for now, consider extracting the shared layout into a unified structure or a separate component that accepts the event logo as a slot or property. This would make it much easier to add future events without copying the boilerplate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/LogoHeader.vue` around lines 104 - 136, Deduplicate the shared logo header structure used by the DEFCON branch and the existing event/default branches. Refactor the layout around the logo glow, separator, title gradient, and tagline fallback into one unified structure or reusable component, passing only the event-specific logo and metadata through props or slots; preserve the existing themeStore.isDark behavior and eventMode.tagline/eventName fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/css/main.css`:
- Around line 341-343: In assets/css/main.css lines 341-343 and 415-417, add an
empty line after each Tailwind `@apply` directive and before the following
background declaration in .btn-primary and .step-badge.
In `@components/LogoHeader.vue`:
- Around line 110-128: Update the three logo image alt attributes in the
template, including both theme variants and the DEF CON image, to use the
component’s existing localization mechanism via translated keys instead of
hardcoded strings. Add or reuse appropriate translation keys while preserving
the current accessible descriptions.
---
Nitpick comments:
In `@components/LogoHeader.vue`:
- Around line 337-355: Replace the responsive height rules for .logo-icon-defcon
with Tailwind classes h-24 sm:h-28 md:h-32 on the DEF CON image in the template.
Move the existing explanatory comment above that image if desired, then remove
the .logo-icon-defcon custom CSS block while preserving the image’s other
classes and attributes.
- Around line 104-136: Deduplicate the shared logo header structure used by the
DEFCON branch and the existing event/default branches. Refactor the layout
around the logo glow, separator, title gradient, and tagline fallback into one
unified structure or reusable component, passing only the event-specific logo
and metadata through props or slots; preserve the existing themeStore.isDark
behavior and eventMode.tagline/eventName fallback.
In `@utils/eventManifest.ts`:
- Around line 166-210: Move the first JSDoc block describing UI theme
application so it immediately precedes applyEventTheme. Keep the
resolveEventAccentVars JSDoc directly above resolveEventAccentVars, with no
unrelated changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fda0890b-2fe3-4b8e-8353-c8d69170382b
⛔ Files ignored due to path filters (1)
assets/img/defcon.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
assets/css/main.csscomponents/LogoHeader.vueutils/eventManifest.test.tsutils/eventManifest.ts
| @apply disabled:opacity-50 disabled:cursor-not-allowed disabled:transform-none disabled:shadow-none; | ||
| background: linear-gradient(135deg, var(--accent) 0%, var(--accent-dark) 100%); | ||
| background: var(--accent); | ||
| color: var(--on-accent); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add an empty line after @apply directives.
To resolve the Stylelint declaration-empty-line-before errors flagged in static analysis, insert an empty line between the Tailwind @apply directives and the standard CSS property declarations.
assets/css/main.css#L341-L343: Add an empty line beforebackground: var(--accent);in.btn-primary.assets/css/main.css#L415-L417: Add an empty line beforebackground: var(--accent);in.step-badge.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 342-342: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
📍 Affects 1 file
assets/css/main.css#L341-L343(this comment)assets/css/main.css#L415-L417
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/css/main.css` around lines 341 - 343, In assets/css/main.css lines
341-343 and 415-417, add an empty line after each Tailwind `@apply` directive and
before the following background declaration in .btn-primary and .step-badge.
Source: Linters/SAST tools
| <img | ||
| v-if="themeStore.isDark" | ||
| src="@/assets/img/logo.svg" | ||
| class="logo-icon" | ||
| alt="Meshtastic Logo" | ||
| > | ||
| <img | ||
| v-else | ||
| src="@/assets/img/logo-dark.svg" | ||
| class="logo-icon" | ||
| alt="Meshtastic Logo" | ||
| > | ||
| </div> | ||
| <span class="logo-separator">×</span> | ||
| <img | ||
| src="@/assets/img/defcon.svg" | ||
| class="logo-icon-event logo-icon-defcon" | ||
| alt="DEF CON 34 Logo" | ||
| > |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use localization for user-visible alt text.
The alt text strings for the logos are hardcoded in the template. As per coding guidelines, all user-visible text must go through useI18n / $t('key') and should not be hardcoded.
Consider replacing "Meshtastic Logo" and "DEF CON 34 Logo" with appropriate translation keys.
🌐 Proposed replacements
- alt="Meshtastic Logo"
+ :alt="$t('meshtastic_logo')"- alt="DEF CON 34 Logo"
+ :alt="$t('defcon_logo')"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img | |
| v-if="themeStore.isDark" | |
| src="@/assets/img/logo.svg" | |
| class="logo-icon" | |
| alt="Meshtastic Logo" | |
| > | |
| <img | |
| v-else | |
| src="@/assets/img/logo-dark.svg" | |
| class="logo-icon" | |
| alt="Meshtastic Logo" | |
| > | |
| </div> | |
| <span class="logo-separator">×</span> | |
| <img | |
| src="@/assets/img/defcon.svg" | |
| class="logo-icon-event logo-icon-defcon" | |
| alt="DEF CON 34 Logo" | |
| > | |
| <img | |
| v-if="themeStore.isDark" | |
| src="`@/assets/img/logo.svg`" | |
| class="logo-icon" | |
| :alt="$t('meshtastic_logo')" | |
| > | |
| <img | |
| v-else | |
| src="`@/assets/img/logo-dark.svg`" | |
| class="logo-icon" | |
| :alt="$t('meshtastic_logo')" | |
| > | |
| </div> | |
| <span class="logo-separator">×</span> | |
| <img | |
| src="`@/assets/img/defcon.svg`" | |
| class="logo-icon-event logo-icon-defcon" | |
| :alt="$t('defcon_logo')" | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/LogoHeader.vue` around lines 110 - 128, Update the three logo
image alt attributes in the template, including both theme variants and the DEF
CON image, to use the component’s existing localization mechanism via translated
keys instead of hardcoded strings. Add or reuse appropriate translation keys
while preserving the current accessible descriptions.
Source: Coding guidelines
The applyEventTheme doc block was left stranded above resolveEventAccentVars when that pure helper was extracted; move it back down. (CodeRabbit #376)
1 participant
What
Gives the DEF CON event mode (
defcon.meshtastic.org/?event=DEFCON) a dedicated branded header and fixes several low-contrast issues in the DEF CON theme on dark mode.Logo
assets/img/defcon.svg, svgo-optimised, transparent so it adapts to both themes) and renders it inLogoHeaderasM × logo, matching the Open Sauce treatment.Accessibility / theming
DEF CON's
primaryis a dark navy (#0D294A) that the theme system was mapping to--accent, so on dark mode the primary button, gradient title, and brand icons (e.g. the device-detail link icon) were nearly invisible.--accent(no gradient) with an auto-contrast--on-accenttext colour (black/white chosen by WCAG relative luminance).applyEventThemenow prefers the theme's dedicatedaccentcolour (DEF CON#E0004E) for--accent, falling back toprimaryfor events that only specify one colour — so Hamvention / Open Sauce / Burning Man are unchanged.--primary-color— the.text-meshtastic/.bg-meshtastic/.border-meshtasticbrand utilities (link/plug icons, brand buttons) — tracks the accent, matching the default theme where the two are already the same colour.Tests
resolveEventAccentVars()and adds unit tests for the accent-selection and on-accent contrast logic.vitestgreen (11/11), ESLint clean.Verification
Verified in the dev server via
?event=DEFCONin dark, light, and mobile: the header logo renders in both themes and the primary button / title / brand icons are all legible.Summary by CodeRabbit