Add Event Details modal showing firmware settings and event info - #389
Conversation
Adds an "Event Details" button to the main button row (event mode only, e.g. the DEF CON flasher) that opens a dismissible sheet showing the event firmware's details: event name, dates, location and time zone, firmware build and version, theme name/tagline/palette, and official event links. The sheet follows the existing pure-Vue dialog pattern (AlphanautFeedback) and closes via the X button, backdrop click, or Escape with focus restored to the trigger. The full manifest edition (dates, location, links, firmware version) was previously discarded after manifestEditionToEventMode(); it is now kept in a new useEventEdition composable, set only alongside setActiveEventMode so the displayed details never diverge from the applied event mode. Event dates are date-only strings, so a small formatEventDateRange util parses them as local dates (avoiding the UTC off-by-one) and is covered by colocated Vitest tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WWbctVTeY3iySKcELxhcR5
Event firmware is compiled from userPrefs.jsonc on the matching event/<slug> branch of meshtastic/firmware. The Event Details sheet now fetches that file lazily on first open (keyed off the edition's firmware slug) and renders the settings flashers actually care about: LoRa region, modem preset, frequency slot, hop limit and ignore-MQTT; each channel with its name, base64 PSK, uplink flag and position precision; and MQTT server settings — plus a link to the source file on GitHub. A new utils/eventUserPrefs.ts holds the pure logic (string-aware JSONC comment stripping, C byte-array → base64 PSK conversion, protobuf enum prettifying, structured extraction) with colocated Vitest coverage, verified against the real event/defcon34 file. Fetch failures degrade to a muted "unavailable" note; static-fallback events without a firmware slug skip the settings section entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WWbctVTeY3iySKcELxhcR5
Drop the event, firmware-build, theme and links cards — the sheet now shows just the settings baked into the event build (LoRa, channels with PSKs, MQTT) plus the userPrefs.jsonc source link. Button and dialog are retitled "Firmware Settings" / "Event Firmware Settings" with a settings icon, and the now-unused date-range util and i18n keys are removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WWbctVTeY3iySKcELxhcR5
Bring back the event, firmware-build and links cards (and the date-range util they use) alongside the radio settings — only the theme name/tagline/palette card stays removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WWbctVTeY3iySKcELxhcR5
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
📝 WalkthroughWalkthroughAdds an event details modal shown while disconnected, shared active edition state, locale-aware event date formatting, and lazy retrieval and parsing of firmware ChangesEvent details flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant EventDetails
participant fetchEventUserPrefs
participant GitHubRaw
participant serialMonitorStore
Visitor->>EventDetails: Open details modal
EventDetails->>fetchEventUserPrefs: Fetch preferences by firmware slug
fetchEventUserPrefs->>GitHubRaw: Request userPrefs.jsonc
GitHubRaw-->>fetchEventUserPrefs: Return JSONC content
fetchEventUserPrefs-->>EventDetails: Return parsed preferences
serialMonitorStore-->>EventDetails: Report serial connection
EventDetails->>EventDetails: Close modal
Possibly related PRs
Suggested reviewers: 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 |
|
@thebentern Took at stab at having claude add a button to view the event details including radio settings (as the def con folks were asking about it). Please review and feel free to scorch everything if it didn't do a good job at it. I'm not sure how to see the preview so I can't tell, the preview claude showed me looked good. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
composables/useEventEdition.ts (1)
9-15: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winExported ref is mutable; consider
readonly().
useEventEdition()returns the raw writable ref. Any consumer could set.valuedirectly, bypassingsetActiveEventEditionand breaking the "set only alongsidesetActiveEventMode, so the two never diverge" invariant documented inplugins/eventMode.client.ts.♻️ Proposed fix
-import { ref } from 'vue' +import { readonly, ref } from 'vue' import type { EventFirmwareEdition } from '~/types/eventFirmware' ... -export const useEventEdition = () => activeEventEdition +export const useEventEdition = () => readonly(activeEventEdition)🤖 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 `@composables/useEventEdition.ts` around lines 9 - 15, Make the ref returned by useEventEdition readonly so consumers cannot mutate activeEventEdition.value directly and bypass setActiveEventEdition. Preserve setActiveEventEdition as the internal mutation path and keep the existing EventFirmwareEdition|null state and API behavior.components/EventDetails.vue (1)
20-25: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoffNo focus trap inside the modal.
Tab/Shift+Tab can move focus out of the dialog into background page content while open, since this is a plain
div[role="dialog"]rather than a natively focus-trapped element. Consider a focus-trap composable (e.g.useFocusTrapfrom@vueuse/integrations) to keep keyboard focus contained while open.As per coding guidelines, "Maintain keyboard and focus behavior for interactive elements; use semantic buttons/links and ARIA labels when appropriate."
🤖 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/EventDetails.vue` around lines 20 - 25, Update the EventDetails modal container to use a focus-trap mechanism, such as VueUse’s useFocusTrap, activated while the dialog is open and deactivated when it closes. Preserve the existing dialog semantics and ensure initial focus is placed inside the modal.Source: Coding guidelines
🤖 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 `@components/EventDetails.vue`:
- Around line 329-347: Update the preference-loading flow around loadPrefs so
cached prefs and prefsState are associated with the firmwareSlug they were
fetched for. When firmwareSlug changes, invalidate the previous loaded or error
state and refetch for the new slug, while preserving the loading guard for the
same slug and avoiding requests when no slug is available. Ensure the reactive
edition change triggers this reset and reload so prefs and prefsSourceUrl
reflect the active firmware.
In `@utils/eventDates.test.ts`:
- Around line 26-29: Strengthen the assertion in the “keeps the calendar day
regardless of the local time zone” test so it verifies the rendered day is 6,
rather than merely checking that the output contains the character “6.” Assert
the expected date text or otherwise inspect the day component, while preserving
the existing formatEventDateRange call.
---
Nitpick comments:
In `@components/EventDetails.vue`:
- Around line 20-25: Update the EventDetails modal container to use a focus-trap
mechanism, such as VueUse’s useFocusTrap, activated while the dialog is open and
deactivated when it closes. Preserve the existing dialog semantics and ensure
initial focus is placed inside the modal.
In `@composables/useEventEdition.ts`:
- Around line 9-15: Make the ref returned by useEventEdition readonly so
consumers cannot mutate activeEventEdition.value directly and bypass
setActiveEventEdition. Preserve setActiveEventEdition as the internal mutation
path and keep the existing EventFirmwareEdition|null state and API behavior.
🪄 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: 2b3e9383-f1af-4f6e-8bd8-053e86da7c8a
📒 Files selected for processing (9)
app.vuecomponents/EventDetails.vuecomposables/useEventEdition.tsi18n/locales/en.jsonplugins/eventMode.client.tsutils/eventDates.test.tsutils/eventDates.tsutils/eventUserPrefs.test.tsutils/eventUserPrefs.ts
| const firmwareSlug = computed(() => edition.value?.firmware?.slug) | ||
| const prefsSourceUrl = computed(() => firmwareSlug.value ? eventUserPrefsSourceUrl(firmwareSlug.value) : undefined) | ||
| const hasLora = computed(() => !!prefs.value && ( | ||
| prefs.value.region !== undefined | ||
| || prefs.value.modemPreset !== undefined | ||
| || prefs.value.frequencySlot !== undefined | ||
| || prefs.value.hopLimit !== undefined | ||
| || prefs.value.ignoreMqtt !== undefined | ||
| )) | ||
| const hasMqtt = computed(() => !!prefs.value && Object.values(prefs.value.mqtt).some(v => v !== undefined)) | ||
|
|
||
| async function loadPrefs() { | ||
| const slug = firmwareSlug.value | ||
| if (prefsState.value !== 'idle' || !slug) return | ||
| prefsState.value = 'loading' | ||
| const result = await fetchEventUserPrefs(slug) | ||
| prefs.value = result | ||
| prefsState.value = result ? 'loaded' : 'error' | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fetched prefs never refresh if the active edition changes.
loadPrefs()'s prefsState.value !== 'idle' guard means prefs/prefsSourceUrl are fetched once and cached for the component's lifetime. If edition (and thus firmwareSlug) later resolves to a different event — plausible given the "background API refresh" logic in plugins/eventMode.client.ts — the modal keeps showing stale radio settings (or a stale error state) instead of refetching for the new slug.
♻️ Proposed fix
+watch(firmwareSlug, () => {
+ prefs.value = null
+ prefsState.value = 'idle'
+})
+
async function loadPrefs() {
const slug = firmwareSlug.value
if (prefsState.value !== 'idle' || !slug) return📝 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.
| const firmwareSlug = computed(() => edition.value?.firmware?.slug) | |
| const prefsSourceUrl = computed(() => firmwareSlug.value ? eventUserPrefsSourceUrl(firmwareSlug.value) : undefined) | |
| const hasLora = computed(() => !!prefs.value && ( | |
| prefs.value.region !== undefined | |
| || prefs.value.modemPreset !== undefined | |
| || prefs.value.frequencySlot !== undefined | |
| || prefs.value.hopLimit !== undefined | |
| || prefs.value.ignoreMqtt !== undefined | |
| )) | |
| const hasMqtt = computed(() => !!prefs.value && Object.values(prefs.value.mqtt).some(v => v !== undefined)) | |
| async function loadPrefs() { | |
| const slug = firmwareSlug.value | |
| if (prefsState.value !== 'idle' || !slug) return | |
| prefsState.value = 'loading' | |
| const result = await fetchEventUserPrefs(slug) | |
| prefs.value = result | |
| prefsState.value = result ? 'loaded' : 'error' | |
| } | |
| const firmwareSlug = computed(() => edition.value?.firmware?.slug) | |
| const prefsSourceUrl = computed(() => firmwareSlug.value ? eventUserPrefsSourceUrl(firmwareSlug.value) : undefined) | |
| const hasLora = computed(() => !!prefs.value && ( | |
| prefs.value.region !== undefined | |
| || prefs.value.modemPreset !== undefined | |
| || prefs.value.frequencySlot !== undefined | |
| || prefs.value.hopLimit !== undefined | |
| || prefs.value.ignoreMqtt !== undefined | |
| )) | |
| const hasMqtt = computed(() => !!prefs.value && Object.values(prefs.value.mqtt).some(v => v !== undefined)) | |
| watch(firmwareSlug, () => { | |
| prefs.value = null | |
| prefsState.value = 'idle' | |
| }) | |
| async function loadPrefs() { | |
| const slug = firmwareSlug.value | |
| if (prefsState.value !== 'idle' || !slug) return | |
| prefsState.value = 'loading' | |
| const result = await fetchEventUserPrefs(slug) | |
| prefs.value = result | |
| prefsState.value = result ? 'loaded' : 'error' | |
| } |
🤖 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/EventDetails.vue` around lines 329 - 347, Update the
preference-loading flow around loadPrefs so cached prefs and prefsState are
associated with the firmwareSlug they were fetched for. When firmwareSlug
changes, invalidate the previous loaded or error state and refetch for the new
slug, while preserving the loading guard for the same slug and avoiding requests
when no slug is available. Ensure the reactive edition change triggers this
reset and reload so prefs and prefsSourceUrl reflect the active firmware.
| it('keeps the calendar day regardless of the local time zone', () => { | ||
| // A UTC-midnight parse would render Aug 6 as Aug 5 anywhere west of UTC. | ||
| expect(formatEventDateRange('2026-08-06')).toContain('6') | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assertion doesn't actually test the timezone-safety it claims to.
"Aug 5, 2026" (the buggy output this test is meant to catch) also contains '6' — from the year. .toContain('6') passes whether the day renders as 5 or 6, so this test can't detect the regression described in its own comment.
🐛 Proposed fix
- expect(formatEventDateRange('2026-08-06')).toContain('6')
+ expect(formatEventDateRange('2026-08-06')).toContain('Aug 6')📝 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.
| it('keeps the calendar day regardless of the local time zone', () => { | |
| // A UTC-midnight parse would render Aug 6 as Aug 5 anywhere west of UTC. | |
| expect(formatEventDateRange('2026-08-06')).toContain('6') | |
| }) | |
| it('keeps the calendar day regardless of the local time zone', () => { | |
| // A UTC-midnight parse would render Aug 6 as Aug 5 anywhere west of UTC. | |
| expect(formatEventDateRange('2026-08-06')).toContain('Aug 6') | |
| }) |
🤖 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/eventDates.test.ts` around lines 26 - 29, Strengthen the assertion in
the “keeps the calendar day regardless of the local time zone” test so it
verifies the rendered day is 6, rather than merely checking that the output
contains the character “6.” Assert the expected date text or otherwise inspect
the day component, while preserving the existing formatEventDateRange call.
Description
Adds an "Event Details" modal dialog that displays comprehensive information about event firmware builds, including:
The modal fetches and parses the
userPrefs.jsoncfile from the event firmware branch on GitHub to extract the radio settings baked into the build. This gives users visibility into exactly what configuration they'll get when flashing event firmware.Implementation Details
SHORT_TURBO→Short Turbo)The modal integrates with existing event mode infrastructure and automatically closes when the serial monitor connects.
Type of Change
Testing
Summary by CodeRabbit
New Features
Bug Fixes