Skip to content

Fix filtering performance#9690

Open
manuelmeister wants to merge 15 commits intoecamp:develfrom
manuelmeister:performance/filtering
Open

Fix filtering performance#9690
manuelmeister wants to merge 15 commits intoecamp:develfrom
manuelmeister:performance/filtering

Conversation

@manuelmeister
Copy link
Copy Markdown
Member

@manuelmeister manuelmeister commented May 5, 2026

This PR improves perceived loading performance in dashboard/program/print views while keeping filter state and result counts consistent.

Changes

  • Move ScheduleEntryFilters endpoint loading responsibility to parent views.
  • Load dashboard/program data in phases so the UI can render earlier without briefly showing unfiltered or incorrectly filtered rows.
  • Keep URL query filters applied before endpoint loading starts.
  • Prevent the dashboard welcome message from flashing during loading races.
  • Make print config local state instead of a computed repair loop to avoid repeated expensive repair/stringify cycles.
  • Reuse already available print store data via _meta.load instead of forcing reloads.
  • Repair/persist print config only when the repaired config actually differs.
  • Delay print previews/PDF preview components until their tab/action is active.
  • Keep dialog filter edits local while the dialog is open and avoid emitting incomplete result counts.
  • Guard against invalid labels causing JS crashes.
  • Reactivate and fix repairPrintConfig tests.

@manuelmeister manuelmeister added the deploy! Creates a feature branch deployment for this PR label May 5, 2026
@manuelmeister manuelmeister temporarily deployed to feature-branch May 5, 2026 18:24 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

Feature branch deployment ready!

Name Link
😎 Deployment https://pr9690.ecamp3.ch/
🔑 Login test@example.com / test
🕒 Last deployed at Wed May 06 2026 14:40:06 GMT+0200
🔨 Latest commit ddad9040c55064eb79aba4a41c090cd077c8236e
🔍 Latest deploy log https://github.com/ecamp/ecamp3/actions/runs/25435571187/job/74613087988
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

@manuelmeister manuelmeister changed the title Performance/filtering Fix filtering performance May 5, 2026
@manuelmeister manuelmeister marked this pull request as ready for review May 5, 2026 18:43
@manuelmeister manuelmeister requested a review from a team May 5, 2026 18:43
Copy link
Copy Markdown
Member

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

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

Any newly added print parts can no longer be filtered until the page is reloaded

@manuelmeister manuelmeister temporarily deployed to feature-branch May 6, 2026 12:39 — with GitHub Actions Inactive
Copy link
Copy Markdown
Member

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

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

Comparing dev and the PR deployment on my phone, the performance seems to have improved. Have not yet tested it without the previews. That could in theory be the bulk of the performance boost, who knows. Also have not yet read through all the code changes. Commit messages seem sensible so far.

@manuelmeister manuelmeister requested a review from a team May 6, 2026 20:07
Copy link
Copy Markdown
Contributor

@BacLuc BacLuc left a comment

Choose a reason for hiding this comment

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

Seems to work, makes it way better. Thank you very much.
We can maybe make it even better, but we can also move that to the next pr.

methods: {
async loadRemainingPrintData() {
const periods = this.camp.periods().items
await Promise.all([
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why don't you start all promises at once?

setRepairedConfig(config) {
const repaired = this.repairConfig(config)
this.cnf = repaired
if (jsonStringifyReactiveValue(config) !== jsonStringifyReactiveValue(repaired)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh no, my hack jsonStringifyReactiveValue spreads like weed.
If it does the job

},
setRepairedConfig(config) {
const repaired = this.repairConfig(config)
this.cnf = repaired
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

inline repaired and use this.cnf in the condition?
Ctrl + alt + n

}
},
async loadEndpointData(endpoint, load = this.camp[endpoint]()._meta.load) {
await load
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Like this you can't start all promises at once.
return load.then(() => this.loadingEndpoints[endpoint] = false).

or even better: load.catch((e) => Sentry....finally(() => this.loadingEndpoints[endpoint] = false)
But we will anyway have the failed request in sentry, maybe the finally is enough.

this.onChange()
}
},
async loadEndpointData(endpoint, load = this.camp[endpoint]()._meta.load) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

using the first paramter to compute the default of the second.
With javascript you can do fancy stuff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy! Creates a feature branch deployment for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants