Skip to content

V31 Persist event display filter#3744

Open
frederikja163 wants to merge 6 commits intoopenfrontio:mainfrom
frederikja163:3739-persist-event-display-filter
Open

V31 Persist event display filter#3744
frederikja163 wants to merge 6 commits intoopenfrontio:mainfrom
frederikja163:3739-persist-event-display-filter

Conversation

@frederikja163
Copy link
Copy Markdown

@frederikja163 frederikja163 commented Apr 23, 2026

If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #3739

Description:

Adds client side persistence to the event display filters.
Not sure if tests are necessary for this. But if they are i will happily add them.

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

Frederikja

Closes openfrontio#3739
Adds client side persistence to the event display filters.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

EventsDisplay now persists per-MessageCategory event filter toggles to browser localStorage, restores them on connectedCallback, and updates localStorage when toggles change, then requests a UI update.

Changes

Cohort / File(s) Summary
Event Filter Persistence
src/client/graphics/layers/EventsDisplay.ts
On component connect, load stored "true" values from localStorage for each MessageCategory into eventsFilters (swallowing storage errors). toggleEventFilter computes the next boolean, updates eventsFilters, writes the boolean string back to localStorage (best-effort), and calls requestUpdate().

Sequence Diagram(s)

sequenceDiagram
  participant User as User
  participant ED as EventsDisplay
  participant LS as LocalStorage
  participant UI as Renderer

  Note over ED,LS: On connectedCallback
  ED->>LS: getItem(categoryKey) for each MessageCategory
  LS-->>ED: "true" / "false" / null
  ED->>ED: set eventsFilters from stored values
  ED-->>UI: request initial render

  Note over User,ED: On toggle
  User->>ED: click toggle(category)
  ED->>ED: compute next boolean, update eventsFilters
  ED->>LS: setItem(categoryKey, "true"/"false")
  LS-->>ED: (ack)
  ED->>UI: requestUpdate()
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

Small toggles sleep in browser light,
Woken on connect, kept through night,
Click once to change, it learns your way—
A little memory for your play. 🎮✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR successfully implements the core requirement from issue #3739: persisting chat notification settings (event filters) across sessions on the client side using localStorage.
Out of Scope Changes check ✅ Passed All changes in the PR are directly scoped to implementing client-side persistence of event display filters as required by issue #3739, with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description clearly relates to the changeset, describing client-side persistence for event display filters that matches the code changes in EventsDisplay.ts.
Title check ✅ Passed The title clearly and specifically describes the main change: adding persistence to event display filters using localStorage, which matches the core functionality added in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 23, 2026
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 23, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/client/graphics/layers/EventsDisplay.ts`:
- Around line 106-118: The persistence uses bare category names and uncaught
localStorage calls; change both loadFilter (called from connectedCallback) and
toggleEventFilter to use a namespaced key like `eventsFilter:${category}`, wrap
localStorage.getItem/setItem in try/catch to handle storage-unavailable errors,
and make connectedCallback iterate over Object.values(MessageCategory) calling
loadFilter(category) instead of five hard-coded calls so new categories are
picked up automatically; ensure loadFilter reads the namespaced key and sets
this.eventsFilters, and toggleEventFilter writes the same namespaced key inside
a try/catch (no-op if storage fails).
🪄 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

Run ID: 55154818-d5a9-4e36-8729-3ca7f016e69d

📥 Commits

Reviewing files that changed from the base of the PR and between d49b1a2 and fe7a6ce.

📒 Files selected for processing (1)
  • src/client/graphics/layers/EventsDisplay.ts

Comment thread src/client/graphics/layers/EventsDisplay.ts
@github-project-automation github-project-automation Bot moved this from Triage to Development in OpenFront Release Management Apr 23, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 23, 2026
@frederikja163
Copy link
Copy Markdown
Author

@iiamlewis this should be ready to merge, not sure if there are tests i need to add or not. But if i am let know i will happily do it.

@frederikja163 frederikja163 changed the title 3739 Persist event display filter V31 Persist event display filter Apr 28, 2026
@ryanbarlow97 ryanbarlow97 added the UI/UX UI/UX changes including assets, menus, QoL, etc. label Apr 29, 2026
@ryanbarlow97 ryanbarlow97 added this to the v31 milestone Apr 29, 2026
@github-project-automation github-project-automation Bot moved this from Development to Final Review in OpenFront Release Management Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

UI/UX UI/UX changes including assets, menus, QoL, etc.

Projects

Status: Final Review

Development

Successfully merging this pull request may close these issues.

Keep chat notification settings

3 participants