Skip to content

Conversation

@Chris-Almond
Copy link
Contributor

Adding the ability for users to see when their profile expires without waiting for the pop-up warning modal message.

This small enhancement gives users more visibility into the remaining life of their Loop app, to help them plan to rebuild and load fresh profiles.

UI Before

beforeLight

After changes

> 3 Days Dark

greaterThan3DaysDark

> 3 Days Light

greaterThan3DaysLight

< 3 Days Dark

lessThan3DaysDark

< 3 Days Light

lessThan3DaysLight

Copy link
Collaborator

@ps2 ps2 left a comment

Choose a reason for hiding this comment

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

Nice! Profile Expiration and LoopDocs are DIY Loop specific things, so it should probably live in LoopSupport, which is a DIY Plugin. If you can move it there, I'd be happy to accept this PR. Another option would be to put it behind a feature flag.

@Chris-Almond
Copy link
Contributor Author

Added a feature flag PROFILE_EXPIRATION_SETTINGS_VIEW_ENABLED in the latest commit with the default as disabled.

  1. How does this look?
  2. Do you prefer I squash these commits together myself (or will GitHub handle that during the merge)?
  3. Do you want me to create a PR adding this flag in a commented out state to LoopWorkspace's LoopConfigOverride.xcconfig? If so, any specific branch I should target? (sample change below)
 // Features
-SWIFT_ACTIVE_COMPILATION_CONDITIONS = $(inherited) SIMULATORS_ENABLED //DEBUG_FEATURES_ENABLED
+SWIFT_ACTIVE_COMPILATION_CONDITIONS = $(inherited) SIMULATORS_ENABLED //PROFILE_EXPIRATION_SETTINGS_VIEW_ENABLED //DEBUG_FEATURES_ENABLED

@Chris-Almond Chris-Almond requested a review from ps2 February 2, 2023 07:17
@ps2
Copy link
Collaborator

ps2 commented Feb 3, 2023

I can squash commits during merge, thanks. Generally the default state is the DIY state, so I think the flag should be such that without any extra compilation conditions, the profile UI is shown. And we can add the flag to disable it in builds where it shouldn't be shown, so you would not need to submit a Workspace PR.

@Chris-Almond
Copy link
Contributor Author

Sounds great. Thanks.

Ok, so my action item is to invert the flag to let the default be enabled.

Anything else or good to go after that?

Copy link
Contributor Author

@Chris-Almond Chris-Almond left a comment

Choose a reason for hiding this comment

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

Inverted the feature flag so the default is enabled.

Also squashed a tiny rendering bug that shows hours when days remaining > 3 and hours == 0.

@ps2 ps2 merged commit 3e72885 into LoopKit:dev Feb 7, 2023
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.

2 participants