-
Notifications
You must be signed in to change notification settings - Fork 24
feat(dashpay): wire the DashConnect connections flow to Dash Platform #909
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
base: develop
Are you sure you want to change the base?
Changes from all commits
42c60d0
b95abb1
96a46f1
4b0bdfc
e184b85
08530f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "info" : { | ||
| "author" : "xcode", | ||
| "version" : 1 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| { | ||
| "images" : [ | ||
| { | ||
| "filename" : "dashconnect-empty.png", | ||
| "idiom" : "universal", | ||
| "scale" : "1x" | ||
| }, | ||
| { | ||
| "filename" : "dashconnect-empty@2x.png", | ||
| "idiom" : "universal", | ||
| "scale" : "2x" | ||
| }, | ||
| { | ||
| "filename" : "dashconnect-empty@3x.png", | ||
| "idiom" : "universal", | ||
| "scale" : "3x" | ||
| } | ||
| ], | ||
| "info" : { | ||
| "author" : "xcode", | ||
| "version" : 1 | ||
| }, | ||
| "properties" : { | ||
| "preserves-vector-representation" : true, | ||
| "template-rendering-intent" : "original" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "images" : [ | ||
| { "filename" : "icon.svg", "idiom" : "universal" } | ||
| ], | ||
| "info" : { "author" : "xcode", "version" : 1 }, | ||
| "properties" : { | ||
| "preserves-vector-representation" : true, | ||
| "template-rendering-intent" : "original" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "images" : [ | ||
| { "filename" : "icon.svg", "idiom" : "universal" } | ||
| ], | ||
| "info" : { "author" : "xcode", "version" : 1 }, | ||
| "properties" : { | ||
| "preserves-vector-representation" : true, | ||
| "template-rendering-intent" : "original" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "images" : [ | ||
| { "filename" : "icon.svg", "idiom" : "universal" } | ||
| ], | ||
| "info" : { "author" : "xcode", "version" : 1 }, | ||
| "properties" : { | ||
| "preserves-vector-representation" : true, | ||
| "template-rendering-intent" : "original" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| { | ||
| "images" : [ | ||
| { | ||
| "filename" : "menu-connections.png", | ||
| "idiom" : "universal", | ||
| "scale" : "1x" | ||
| }, | ||
| { | ||
| "filename" : "menu-connections@2x.png", | ||
| "idiom" : "universal", | ||
| "scale" : "2x" | ||
| }, | ||
| { | ||
| "filename" : "menu-connections@3x.png", | ||
| "idiom" : "universal", | ||
| "scale" : "3x" | ||
| } | ||
|
Comment on lines
+2
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Replace the raster menu glyph with an SVG asset. This new generic icon uses PNG files at three scales. Use a local SVG with As per coding guidelines, “Prefer SVG over PNG for new icons, preserve vector representation, and use the correct template or original rendering intent for the consuming UI.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| ], | ||
| "info" : { | ||
| "author" : "xcode", | ||
| "version" : 1 | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "images" : [ | ||
| { "filename" : "icon.svg", "idiom" : "universal" } | ||
| ], | ||
| "info" : { "author" : "xcode", "version" : 1 }, | ||
| "properties" : { | ||
| "preserves-vector-representation" : true, | ||
| "template-rendering-intent" : "original" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use template rendering for generic SVG UI icons.
Both assets use
"original"despite being generic UI glyphs. This prevents the consuming UI from applying its tint.DashWallet/Resources/AppAssets.xcassets/DashConnect/dashconnect.xmark.circle.imageset/Contents.json#L6-L9: set"template-rendering-intent"to"template".DashWallet/Resources/AppAssets.xcassets/DashConnect/menu.connections.imageset/Contents.json#L6-L9: set"template-rendering-intent"to"template".As per coding guidelines, “use the correct template or original rendering intent for the consuming UI.” Based on learnings, generic SVG UI icons must use
template.📍 Affects 2 files
DashWallet/Resources/AppAssets.xcassets/DashConnect/dashconnect.xmark.circle.imageset/Contents.json#L6-L9(this comment)DashWallet/Resources/AppAssets.xcassets/DashConnect/menu.connections.imageset/Contents.json#L6-L9🤖 Prompt for AI Agents
Sources: Coding guidelines, Learnings