WIP: Add identicon into profile qr code #10703
Conversation
|
Would you mind sharing a sample of the QRs that are produced with this change? Thank you 😄 |
There was a problem hiding this comment.
Looks great on my end! Tried on Android. @errorists can you also take a look please?
|
|
| :height logo-size | ||
| :stroke "blue" | ||
| "stroke-width" 2 | ||
| :rx (/ logo-size 2) |
There was a problem hiding this comment.
can we move (/ logo-size 2) to let? It is repeated four times
| (let [path (reagent/atom nil)] | ||
| (.toString qr-code-js | ||
| value | ||
| (bean/->js {:margin 0 :width size :errorCorrectionLevel "H"}) |
There was a problem hiding this comment.
Can we either have only one key/value pair per line or separate pairs with comma? It is a bit hard to read for me personally
There was a problem hiding this comment.
I totally agree with you, and I would prefer to have that as a lint rule. We have many places where it's multiple pairs per line, and IMHO it's even harder to read when it's multiple pairs per line and multiple lines for the same map.
|
Small request - could you please rebase to latest master so this commit b5fda12 gets included? This means we aren't running E2E tests on eth.prod cluster anymore, and since this happens on every push, the impact is quite big. This ensures we have more accurate metrics going forward from Jul 1 onward, which would be awesome. See https://discuss.status.im/t/user-growth-and-retention/1782 for more |
|
For reference: This PR is part of epic #10502 and fixes #10627 PR is currently WIP, using compressed key not implemented yet, method available in status-go see https://specs.status.im/spec/2#public-key-serialization and status-im/status-go#1990. Issue to implement use of compressed key in status-react here: #10325 |
|
Should be done in Go as it should be shared between all platforms |
Added only to profile for now