Skip to content

Conversation

@marv-out
Copy link
Contributor

Add treatment-type filtering with custom popover in Treatment Root View

Summary

This PR introduces filtering for treatment types in the Treatment Root View.
A custom popover lets users select or deselect which treatment types to display.
The implementation is intentionally simple in both code and UI.

Details

  • Adds a custom popover to toggle visibility of individual treatment types
grafik

Notes / Caveats

I wrote the initial version a while back and haven’t thoroughly re-tested it since.
There may be edge cases I’m not remembering.
The main area I considered improving was the UI polish; after a quick look today it still seems acceptable, but I’m open to suggestions.

Testing Requested

  • Verify that selecting/deselecting types immediately updates the list
  • Check that filters behave correctly across navigations within the view

Suggestions and feedback are very welcome.

marv-out and others added 20 commits June 15, 2025 17:56
@Sjoerd-Bo3
Copy link
Contributor

Thanks mate! For uploading

@Sjoerd-Bo3
Copy link
Contributor

Sjoerd-Bo3 commented Oct 20, 2025

Works great. 1 nitpick: would be nice if the 'Select All' would also deselect everything when everything is selected already. That kind of seems intuitive and logical UX wise IMO.

Looks great, also deglassified:
image

@dnzxy
Copy link
Contributor

dnzxy commented Oct 22, 2025

This LGTM.

@marv-out would you like to address the comment about de-selecting entries when double tapping Select All or keep it as is?

@marv-out
Copy link
Contributor Author

Yep, I can implement this later today. It does make sense

@marv-out
Copy link
Contributor Author

updated the functionality, but I noticed a plist change when building with xcode 26 again. I haven't committed that now, not sure if thats correct or not...

+++ b/Trio/Resources/InfoPlist.xcstrings
@@ -457,6 +457,18 @@
         }
       }
     },
+    "NSCalendarsFullAccessUsageDescription" : {
+      "comment" : "Privacy - Calendars Full Access Usage Description",
+      "extractionState" : "extracted_with_value",
+      "localizations" : {
+        "en" : {
+          "stringUnit" : {
+            "state" : "new",
+            "value" : "To create events with BG reading values, so that they can be viewed on Apple Watch and CarPlay"
+          }
+        }
+      }
+    },
     "NSCalendarsUsageDescription" : {
       "comment" : "Privacy - Calendars Usage Description",
       "extractionState" : "extracted_with_value",
       ```

@MikePlante1
Copy link
Contributor

updated the functionality, but I noticed a plist change when building with xcode 26 again. I haven't committed that now, not sure if thats correct or not...

I think the best way is to just build with Xcode 16.4 and then commit before pushing, even if you do the actual coding in Xcode 26

If you don’t do that, though, do what you did and don’t include that change in the commit.

@marv-out
Copy link
Contributor Author

Ok, then it should be good to go. I don't have Xcode 16.4 anymore...

@MikePlante1
Copy link
Contributor

I build it via Xcode 16.4 and the only fix-it is re-inserting a stale l18n that your PR removed. I can push that soon.

Works great on my in-vivo iPhone 14 Pro Max (iOS 26.1 (23B5073a)). Love it!

Works great on an iPhone 17 Simulator (iOS 26.0)

Screen.Recording.2025-10-24.at.11.42.14.mov

Seems to often hide the menu in an iPhone 15 Simulator with iOS 17.5, though.

Screen.Recording.2025-10-24.at.11.35.18.mov

Copy link
Contributor

@MikePlante1 MikePlante1 left a comment

Choose a reason for hiding this comment

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

Tested it on an iPhone SE (2nd gen) Simulator with iOS 18.5, and it works just as great is it does with iOS 26.

So I'm just going to approve this PR as-is. At worst, iOS 17 users will just have to tap to open the filter button a few more times, but also anyone running iOS 17 should have already updated to iOS 18 a while ago as all phones that support iOS 17 also support iOS 18.

@MikePlante1
Copy link
Contributor

It just occurred to me that since the UI looks the same whether treatments are filtered or showing all, it could lead to somebody applying a filter but then not remembering the next time the open History and presuming they're seeing everything when they aren't.

Not sure the best way to present that info in the UI, though.

@Sjoerd-Bo3
Copy link
Contributor

It just occurred to me that since the UI looks the same whether treatments are filtered or showing all, it could lead to somebody applying a filter but then not remembering the next time the open History and presuming they're seeing everything when they aren't.

Not sure the best way to present that info in the UI, though.

Yeah when a filter is active some indication should be shown in the list indeed. Good catch!

@dnzxy
Copy link
Contributor

dnzxy commented Oct 25, 2025

Basing this on Mike's recording.

I'd suggest if there are no entries to display an empty row with something like "No entries" or "No Data" or some similar text.
A completely empty list with just the filter as header row looks very odd.

I agree with the notion of active filters and would suggest to display this with an inverted filter icon (so the current chosen icon becomes inverted, to a fill one) and/or add a small badge with the # of selected/applied filters.

Rest looks and feels wonderful.

@marv-out
Copy link
Contributor Author

I am heading to vacation right now without a mac, so I am not really able to edit sth for the next week. Maybe somebody else could add the icon so that it can be merged

@MikePlante1
Copy link
Contributor

I am heading to vacation right now without a mac, so I am not really able to edit sth for the next week. Maybe somebody else could add the icon so that it can be merged

I'll take a stab at it. Have a great vacation!

@MikePlante1
Copy link
Contributor

How's this?

Screen.Recording.2025-10-25.at.09.42.39.mov

Changed filter button to show filled icon and count when not all treatment types are selected. Updated event display logic for No Data to use filteredPumpEvents instead of pumpEventStored.
@MikePlante1 MikePlante1 marked this pull request as draft October 25, 2025 14:34
@marv-out
Copy link
Contributor Author

I like it :)

@Sjoerd-Bo3
Copy link
Contributor

I agree, looks clear and great like this.

Introduced a TreatmentType enum to replace hardcoded treatment type strings in DataTableRootView. Updated filter logic and UI to use the enum, improving maintainability and reducing duplication.
@MikePlante1 MikePlante1 marked this pull request as ready for review October 26, 2025 02:08
@marv-out
Copy link
Contributor Author

Nice addition, I think its good to go

Copy link
Contributor

@Sjoerd-Bo3 Sjoerd-Bo3 left a comment

Choose a reason for hiding this comment

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

LGTM, good to go!

@Sjoerd-Bo3 Sjoerd-Bo3 merged commit 604114c into nightscout:dev Oct 26, 2025
3 checks passed
@g5WS
Copy link

g5WS commented Oct 27, 2025

Suggestions and feedback are very welcome.

Love this feature, @marv-out! Great job!! 👏🎉😁

mountrcg added a commit to mountrcg/Trio that referenced this pull request Oct 31, 2025
Merge pull request nightscout#818 from marv-out/filtering-treatments
mountrcg added a commit to mountrcg/Trio that referenced this pull request Nov 3, 2025
Merge pull request nightscout#818 from marv-out/filtering-treatments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants