Repository navigation
feat(labs): add biomarker filtering and sorting - #1370
Conversation
MBombeck
left a comment
There was a problem hiding this comment.
Thanks a lot for this, @ferges123. A filter rail is what the labs list needs once it grows past a screenful, and the translations in all ten languages are a real piece of work. I've approved the CI runs.
I'd like to take it. A few things first, roughly in order of how much they matter:
- Groups collapse while filtering. With
groupsOpen: "flagged",openByDefaultis computed from the filtered rows. Filter for "in range", or search for an unremarkable marker, and you get closed headers while the counter says there are matches. The browser test atlab-list-url-sync.browser.test.tsx:520pins that. With any filter active, every group that has a match should open. - The date filter only looks at the latest reading (
lab-list-filters.ts:196-201), but the label says "Date range". A marker measured inside the range and again later drops out. I'd rename it to "Last measured" so it says what it does. - The sort pill. Settings > Labs already has a saved sort order (
labs-settings.tsx). The pill adds a second one that is URL-only and lost on the next visit. Also,isFilteredignores it but "Reset" resets it. I'd drop the pill and keep the list sorted the way Settings says. - The API description in
src/lib/openapi/routes/labs.tsloses the status rule and the sentence that a client never rebuilds category, order or status. The iOS app relies on that wording. Please keep it as it was and add a line saying a filtered subset is summarised by the same rule. src/lib/module-list-prefs.ts: theentry?.savedcheck and the legacy-preference precedence change all three lists (labs, illness, checkups), not just this one. It may well be right, but it's a separate change. Could you split it into its own PR with a sentence on what it fixes?- Small ones: please drop the
/plans/line from.gitignore, since it's local to your setup. Use "…" in the search placeholder like the rest ofen.json. Move theUrlFilterSyncOptionsinterface above the hook's JSDoc inuse-url-filter-sync.ts, so the comment documents the hook.
You don't need to bother with formatting or with the conflicts with the next release. Prettier flags eleven files, and release/v1.47.2 adds a saved category order (categoryOrder) to the same preferences. I'll resolve both when I merge. Your category filter already reads the server's ordered categories, so the two fit together.
If you'd rather I make these changes myself on top of your commit, just say so. Your authorship stays either way. If I haven't heard back by Friday, I'll take it from there so it makes the next patch.
|
@ferges123 I went ahead and did the review changes myself, so this makes it into 1.47.2, which ships tomorrow. Your commit is merged as it is into the release branch (release/v1.47.2, merge f20efca), and this PR will show as merged once the release lands on main. On top of it: groups with a match stay open while filtering, the date filter is now called "Last measured", the sort pill is gone (the saved sort from Settings applies), the API description is back with one added sentence, and the list-preference precedence change is left out for now. I also moved the search field beside the filter rail for phones, fixed typed characters getting lost while the URL caught up, kept focus in the field around clear and Thanks a lot for this one. It's a big, careful piece of work, and the tests made the follow-ups easy. You'll be in the release notes. |
|
Thanks! I've just started using your app and hadn't noticed the sort and grouping option earlier. Thanks for a great application. |
|
@ferges123 This shipped in v1.47.2 and this PR now shows as merged with your commit. Thanks again for the work and the tests. |
Summary
Validation
pnpm typecheck: passed.