Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use environment for appName and replace hardcoded Loop with appName #120

Merged
merged 6 commits into from
Apr 24, 2024

Conversation

marionbarker
Copy link
Collaborator

@marionbarker marionbarker commented Apr 10, 2024

  1. Add code needed to use environment that ties appName to CFBundleDisplayName
    • One instance of appName already present in UncertaintyRecoveredView.swift now updates with MAIN_APP_BUNDLE_IDENTIFIER
  2. Replace the hard-coded "Loop" string with appName
    • There are 3 files that use hard-code "Loop" in strings
    • This PR updates 2 of them so far

Test:

  • Configure override with MAIN_APP_BUNDLE_IDENTIFIER = TestAppRename
  • With the exception of the strings in the one file I could not modify, all strings that used to display "Loop" were confirmed to display "TestAppRename". See later comments about the one file (BeepPreference)

@marionbarker marionbarker marked this pull request as draft April 12, 2024 02:34
@marionbarker
Copy link
Collaborator Author

marionbarker commented Apr 12, 2024

The open PR 118 will modify some of these strings. Hold off on this for now.
Note to self - update for appName

  • BeepPreference.swift

@marionbarker
Copy link
Collaborator Author

Now that PR #118 is merged, I revisited this PR.

  • I have not solved the issue for BeepPreference and need help to fix it
  • Or permission to modify the language to remove hardcoded "Loop" from the strings

The current version of BeepPreference is picking up the default value from LoopKit - details below.

Details:

To test this:

  • I modified MAIN_APP_BUNDLE_IDENTIFIER = TestAppRename in LoopConfigOverride file
  • I modified the LoopKitUI/Extensions/Environment+AppName.swift file to have defaultValue = "LoopKitDefaultValue"
    • Normally defaultValue = "Loop"

UncertaintyRecoveredView.swift works as expected:

  • If I trigger an uncertain delivery and then recover, the string displayed for appName is TestAppRename

BeepPreference.swift is not working as expected:

  • When I look at the Confidence Reminder screen, the value from LoopKit is inserted (LoopKitDefaultValue instead of TestAppRename)
  • Graphic below

I am still missing a step.

omnible-pr-120-not-working

@marionbarker marionbarker marked this pull request as ready for review April 14, 2024 19:58
@marionbarker
Copy link
Collaborator Author

Pushed a commit that makes this "good enough". Should someone be able to get the correct link to appName in BeepPreference, this is an easy change.
In the meantime, set appName to "the app". See graphic below.

omnible-pr120-appName-change-string

// This picks up the DefaultValue from LoopKit, not the CFBundleDisplayName
//@Environment(\.appName) var appName
// ToDo - insert the appName properly
let appName: String = "the app"
Copy link

Choose a reason for hiding this comment

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

Why not use the environment with the real app name? If you have questions about how to set it, I can help. If you want to use "the app", it would be better just to hard code it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please help. I prefer appName.

@marionbarker
Copy link
Collaborator Author

When I tested it, it didn’t work. It used the defaultValue from LoopKit. See a previous comment where I demonstrated this.

@marionbarker
Copy link
Collaborator Author

@ps2 I know I went ahead and merged this. But will happily update with a new PR if you provide help in making it work to use appName from the environment.

To reiterate the problem, this picks up the defaultValue from LoopKit instead of the appName from the environment that I expected. I prefer to fix this the "right" way.

Perhaps it is a LoopKit/LoopKit PR that is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants