reez
commented
6 days ago Thanks for the side-by-side screenshots!
Replaces generic bitcoin icon with the BDK logo used in the app icon
Adds the 'BDK Wallet' name
Expands on the project description 'An example bitcoin wallet...'
App tries to not cut off text, especially on non-DynamicType with regular iPhone screen sizes, here is a screenshot of proposed changes:
App tries to support Dynamic Type but proposed UI does not work well on multiple iPhone devices, here is a screenshot of proposed changes:
Note: BDK logo and app name were not added to this screen because on bi-weekly bindings calls we had conversations about minimizing usage of BDK name and logos (open to continued discussions on this nuanced discussion, hard to distill those conversations in a short comment here, maybe best path forward is discussing on one of the BDK calls so clearest explanation with all the factors is laid out properly)
Changes styling of selectors for network and server options, added titles
App tries to support Dynamic Type but proposed UI does not work well on multiple iPhone devices, here is a screenshot of proposed changes:
Uses bitcoin orange for primary button
Proposed change would reverse https://github.com/bitcoindevkit/BDKSwiftExampleWallet/commit/d9dca01b5ecb4f988f5ad5fc04b7140b7e3be5f2 which probably deserves expanded discussion, and if this proposed change was to be made it should concurrently be made in multiple screens across the app for cohesiveness of color of the similar buttons across the app
Uses Sentence case
https://developer.apple.com/design/human-interface-guidelines/writing “Title or sentence case. Decide whether you want to use title case or sentence case for alerts, page titles, headlines, button labels, and links. Throughout the HIG, you’ll find guidelines for specific components, but how you format your text is a reflection of your app’s voice. Title case is more formal, while sentence case is more casual. Choose a style that fits your app.”
https://developer.apple.com/design/human-interface-guidelines/buttons “Consider using text when a short label communicates more clearly than an icon. To use text, write a few words that succinctly describe what the button does. Using title-style capitalization, consider starting the label with a verb to help convey the button’s action — for example, a button that lets people add items to their shopping cart might use the label “Add to Cart.”
I’m definitely not against apps that use Sentence case (one of my favorite bitcoin apps uses it), and don’t think Apple even thinks one is always better than the other, but it does seem like the strongest case for which would be the standard for an iOS wallet would be Title (per combination of HIG+Wallet)?
Considerations for further improvements
Happy to have an Issue opened up for any/all of these to have a discussion about them!
Other
Did not format code per checklist:
Would encourage continuing discussion on each of these as separate Issues/PR's because this PR discussion might get too large and span too many different topics (and possibly opening up Issues before PR's for these items)
Would love your help on making sure Dynamic Types works across all views for existing UI, there's one small existing issue I know of https://github.com/bitcoindevkit/BDKSwiftExampleWallet/issues/245
I appreciate the experimentation on the OnboardingView!
Description
The UI tweaks in this PR uses the BDK logo and name to create an onboarding screen consistent with the app icon and name as well as general styling improvements.
Notes to the reviewers
Considerations for further improvements