Color selection for Contact Image #349
Conversation
Added color picker
|
Thank you for your submission. All PR for LoopFollow should be against dev branch. Each PR will be tested/approved before merge to dev. The timing of the subsequent release to main depends on circumstances. |
|
Thanks @marionbarker. I wasn't sure if I should submit to Main or Dev. Should I redo this for the Dev branch? |
|
Sorry. I should have been more clear. I already changed the target branch to dev for you. (Anyone who created a PR can also tap edit and change the target branch.) I was able to do it for you because I have write permission for this repo. We are always happy to have new contributors, and sometimes make suggestions for future work. |
| @@ -68,10 +83,10 @@ class ContactImageUpdater { | |||
|
|
|||
| let maxFontSize: CGFloat = extra.isEmpty ? 200 : 160 | |||
| let fontSize = maxFontSize - CGFloat(bgValue.count * 15) | |||
|
|
|||
There was a problem hiding this comment.
Unintended whitespace change.
| @@ -1,4 +1,4 @@ | |||
| // | |||
| // | |||
There was a problem hiding this comment.
Unintended whitespace change.
| @@ -33,6 +33,7 @@ struct ContactSettingsView: View { | |||
| requestContactAccess() | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Unintended whitespace change.
|
Hi! Thanks for the contribution—it’s much appreciated! I made comments on the code regarding some unintended whitespace changes. It would be great to clean those up to keep the diff minimal and focused on the functional changes. I also noticed that some of the colors, for example the purple and blue, appear quite dark and are a bit hard to read (at least on my watch and with my eyes ;) ). Is this something you’ve noticed as well? Perhaps we could tweak the colors to improve readability. I’d like to hear your thoughts on this. Other than that, everything looks great! The code works as expected, and it aligns well with the patterns in the surrounding code. Nice work! 👍 |
|
Yes - I like that approach! The contact card is definitely smaller on my wife's watch than my larger screen so I expect she'd like having the number and the delta separated. I like your simplified recommendation - it makes a lot of sense to me. |
|
Hi @bjorkert - I've finished working on the updates to use additional contact cards. This should allow for easier to read BGs including trend and delta on the Apple Watch. Could you please take a look and provide your feedback? In the Contact Settings, there's now options to select background and text color along with three options for showing the trend arrow and/or the delta. Both can be "Off" or "Separate" but only one can be "Include" where it combines the BG and the trend/delta. This operates just like the previous version except there are now three contact cards that users can choose to add to their watch face: LoopFollow - BG, - Trend, and - Delta 
Please let me know what you think! |
|
Hi! Thanks for the update! I'll try to look into this soon! |
|
mmol values should always be displayed with a decimal. Here is an example with a zero delta, should be +0,0 |
|
The delta is a bit too big with mmol, it should not "touch" the circle. |
|
I think there should be a short description for the separate contact cards, so the user know that another contact card should be selected? |
|
There is a merge conflict, but that is easily solvable in this case. Other than mentioned above, it looks good. I will use the delta in a separate card myself. |
|
Thanks for the feedback @bjorkert! I'll make these changes in the next few days. |
Added a color selector in the Contact settings so individuals can choose different colors based on their watch faces and their preferences. Also should allow individuals with multiple loopers to be able to assign a different color to each. I have tested this on my phone and watch but before merging into the main branch, this new code will benefit from additional code review and/or testing. Thank you!