Spooky UI with QR codes #36
Conversation
|
if we output SVG instead of png we 1. don't have to host temp files on server, just serve and 2. can color it with css please help review / approve #30 so we can focus on changes here independently. Looks like some changes here are formatting. Please keep commits pared down to the logic changes only and formatting changes in their own commits |
|
Not sure what's going on with that conflict, will fix |
nickfarrow
force-pushed
the
mvp-ui
branch
3 times, most recently
from
4fcb0d1 to
6b05463
Compare
|
Got rid of the SVG fixup because it did not work as a background and is not super important |
nickfarrow
force-pushed
the
mvp-ui
branch
2 times, most recently
from
863d9c4 to
48598a3
Compare
| /// Create QR code and save to `STATIC_DIR/qr_codes/<name>.png` | ||
| fn create_qr_code(qr_string: &str, name: &str) { | ||
| let filename = format!("{}/qr_codes/{}.png", STATIC_DIR, name); | ||
| qrcode_generator::to_png_to_file(qr_string, QrCodeEcc::Low, 512, filename.clone()) | ||
| .expect(&format!("Saved QR code: {}", filename)); | ||
| } |
There was a problem hiding this comment.
I'm not sure why we're saving all these QR codes beyond convenience. Also note if this interface were customer facing instead of an internal dashboard, saving these to our public directory exposes the whole history of our addresses to those customers.
I think that's a privacy regression, but I'm still inclined to merge because we want to get in the Umbrel Store for one's own node management first.
When I spent a few minutes trying to get the SVG string spit out I ran into a little trouble, but I think that's the best way going forward because
- unbounded physical size
- no additional data saved to our server
- therefore, no privacy leak
- can be styled ad nauseum with css
There was a problem hiding this comment.
Agreed. Let's make this an issue for now?
One minor positive is that I can't access the raw /static/qr_codes/ dir, the server only seems to serve the files -- perhaps preventing leaking addresses unless they spend crazy amount of time bruteforcing using onchain-used addresses.
The other thing is that this is covered by our current (bad) security assumption that the nolooking .onion is private
static/index.html
Outdated
| <link rel="stylesheet" href="/pj/static/style.css"> | ||
| <link rel="stylesheet" href="style.css"> | ||
| <title>nolooking // Lightning PayJoin</title> | ||
| <style> | ||
| .invisible { | ||
| display: none; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
These styles should be in one file. I see style.css is linked twice because
The root of the problem looks like we're serving static files out of /pj/static instead of root so the first line should be correct
chrome really hates the double and just does not load a style.css
There was a problem hiding this comment.
Yeah that is the issue. Any thoughts on removing the /pj endpoint alltogether?
I'm not great at these relative directories for websites -- do you know what would fix this for both running locally and on umbrel builds?
Does having double break it for you? One of the two loads for me, the other 404s, but the style renders
static/style.css
Outdated
| .styledlink { | ||
| color:rgb(218, 160, 0); | ||
| } |
There was a problem hiding this comment.
alter a theme color & then apply that theme's class so we can re-use existing classes
DanGould
force-pushed
the
mvp-ui
branch
3 times, most recently
from
42610ca to
3fb460d
Compare
|
This is great work and I'm glad you've been sticking with it. Let's get this umbrel branch into master. I hope you don't mind my rebase. I applied the changes I requested. Changes unify styles into styles.css, and fix the inputs being cleared, and spruce up Commit Messages in the style bitcoin/bitcoin and rust-bitcoin/rust-bitcoin use |
|
There are a few UI things driving me insane that we can worry about go in after we get this merged 1. the biggest button isn't the submit button. I think a simple [+] would be intuitive enough. 2. no way to delete excess channel rows. Maybe that's less of a problem if they're not "required" fields. 3. Anchor reserve, but that's being addressed by getting removed |
|
Ahh I didn't see your force pushed changes. Just added the red text color back and fixed the buttons a bit (+made them all If you approve, I will squash this into 1 commit prior to merge but will leave as separate commits for now so you can see changes |
|
I don't think we should squash this one because it's got logic & formatting that should stay separate commits Your new commit message fails! Close, but no cigar. Let's try again
|
|
I mean squash the style commits into one (have commits: HTML+css, QR code). So without that commit message |
Separate configuration and channel queue forms. Channel queue form is now fixed so user input is persistent by using "appendChild". Add a spooky user interface with DALLE background pumpkins. Add emojis and placeholder form text. Provides link to suggest payjoin wallets..
Use qrcode_generator crate to generate and store QR codes as `STATIC/qr_codes/<address>.png`. These QR codes are fetched to display on the frontend using the address from the BIP21 uri.
Builds on top of #30
Squashed from original PR: DanGould#1
"/pj/schedule"creates a qr code inqr_codes, as<address>.pngwhich is retrieved on the website and shown.Not sure if we can do the styled QR code as easily as we could with the other library, we can try some CSS