-
Notifications
You must be signed in to change notification settings - Fork 98
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
Silent payments in the Guide #1109
base: master
Are you sure you want to change the base?
Conversation
Created the file and made cursory changes
Adding all basic content from doc plus misc stuff
Added images and editing text appropriately.
Adding images plus misc
Adding header image and silent payments item to how it works intro page
✅ Deploy Preview for bitcoin-design-site ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Great start with this page. I know you're still working on things, so I didn't go into a ton of detail.
Generally, a lot of content here expands on stuff we already have dedicated pages for (contacts, backup...). As a reader, it can be tricky to piece it all together. Eventually, I think it would be good to have silent payments be covered in those pages, instead of having this addendum. For example, the contacts page should just include them by default. For now, I think it might be good to start paragraph by referencing those other pages, and then only describing what is different.
It's one of those tricky editorial things about the guide, having standalone content, but still having it all work together nicely with little duplication.
Anyhow, let my know if there's any other feedback you'd like (or not like).
Also updated backup section
plus gratitude etc.
Thanks for reading through @GBKS, I see some great feedback, which I intend to add on top a lot of changes I'm about to push (edit: shown above since they were committed yesterday). Agree re: editorial things, see below.
Overall, I agree with you but view this effort as silent payments-focused for people who want to read about silent payments and their UX impact. My view (related to #1101 ) is that reference designs should outline established UX patterns and best practices, which IMO doesn't apply to silent payments currently since it is nascent at this point. That said, I've tried to link to existing pages where I could (do let me know if I missed someplace). |
updates mostly to backup/recovery and receiving/scanning sections
Just updated header images...if those are fine I think this one is ready for final reviews. |
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.
Great progress! The page reads much better already. I made some more suggestions.
It could also be a good idea to add a little bit more structure to the content by framing the bottom half of the content as UX implications. Like so:
- Rationale
- How it works
- New conceptual model
- Labels
- User experience implications
-- Contacts
-- Sending
-- Receiving
-- Setup
-- Backup
-- Recovery - Pro's and cons
I was just about to start reviewing the content, but I see 44 unresolved conversations from previous feedback. @yashrajd, could you please address those before we review again? Did a very quick image check, and please review those as well. Pulling up the site on mobile and quickly scrolling down I saw two broken/missing images. Did not dig any further. I started an image checklist PR earlier with things to look out for (it's a draft and incomplete, but it's a starting point. |
Vast majority of these were addressed (even commit mentioned long time ago) but I had not marked them resolved. Have done so now.
I have investigated this and did not find the reason they're working on mobile, these 2images follow spec AFAICT, perhaps you can help me figure it out... I'm happy to investigate further myself but don't want that to prevent you from reviewing...
Great to see the PR... |
Also fixes the wrong modal image in receive section
missing mobile images are now fixed, *thx Christoph for the help: 8c2b78c |
Not exhaustive, a couple might be added later. Co-authored-by: Michael Haase <[email protected]>
:/ how can I be wrong :P Co-authored-by: Michael Haase <[email protected]>
Also fixed how sp works illustration with correct height
…nto silent-payments
The checklist from #1117 : Front matter
Content
Images
|
Remove unused backup carousel and fix caption on the recovery carousel
Plus phrasing improvements...
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.
Thank for pushing this PR. This review has 30 more comments, but it really is mostly small stuff. Many of them are for adding commas to run-on sentences that are hard to read.
I still find the diagrams hard to read on mobile. You basically have to zoom in and pan around. Up to you if you want to address that further.
The tables are also not great, you basically get one word per line on mobile. Personally I'd just reformat those to regular text and full sentences. The table format requires me as a reader to piece things together. If you give me a sentence, you give me fully formed explanations without gaps. Also up to you what path to go there.
|
||
With the improvements to contacts, on-chain send flows may start from a number of [entry points](/guide/daily-spending-wallet/sending/#payment-entry-points). When users start with obtaining a static address, every on-chain address derived from it will be visibly different from the static address the sender started with. This is likely to be confusing for users. Applications should take measures such as short explainers to avoid confusion on the users part. Test transactions are another good way to help users deal with this. | ||
|
||
{% include image-gallery.html pages = page.images_send %} |
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.
I wonder if some of those tips can be hidden behind an info button and appear in a modal? The text just gets super tiny and that small address is basically impossible to compare.
Thanks Christoph for going through the PR again... Co-authored-by: Christoph Ono <[email protected]>
Thanks @GBKS for the inputs.
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.
ACK. Let's get this one out the door.
This is a draft pull request for silent payments content in the guide. It is ready for review but not ready to merge just yet even if feedback is fine. I was able to able to create server and preview all of the content on local.
Preview the silent payments page
Listing a few minor of to-do's below, apart from the any changes necessitated by feedback:
Look forward to review and feedback by way of suggestions, corrections for screens and content keeping in mind the above to-dos, which I will complete in a few days. I would discourage inputs on approach unless they were really necessary. I wanted to put this out there so the wider community in general could preview & review it.