Skip to content
This repository has been archived by the owner on May 22, 2021. It is now read-only.

Qr code download link proposal #1328

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

oliver-dvorski
Copy link

@oliver-dvorski oliver-dvorski commented May 7, 2019

In hopes of bumping #423 a bit, I figured why not try to implement that feature myself.

Since I have no idea what I'm doing and since I've never heard of Choo before diving into this, I tried oversimplifying the whole thing and coupled a QR code view to the copy to clipboard action. This is what that looks like:

screencast-localhost-8080-2019 05 07-21-14-06

Any thoughts? I'm expecting to trigger everyone with the way I wrote this so don't hold back, just let it out

Sorry for the broken gif, it's late and I want to go home 😄

@dannycoates
Copy link
Contributor

Nice start @oliver-dvorski! We'll want to tweak the ui some, but this is great. I'll be able to give you more feedback next week.

@oliver-dvorski
Copy link
Author

@dannycoates Awesome 😊
Can't wait to refine this

@cmpadden
Copy link

Hi @oliver-dvorski, while I don't have any feedback on this pull request I wanted to say thank you for your proposal.

I often find myself wishing that the Firefox Send URLs were either shorter or had an easy way to copy them to other devices -- this would be great! Thanks!

@oliver-dvorski
Copy link
Author

@cmpadden Thanks! That means a lot. I too can't wait to get this rolling so I can finally kill makeazip.com 😄 and let the big boys provide a much better service than I ever could

app/routes.js Outdated Show resolved Hide resolved
@cjbarth
Copy link

cjbarth commented Oct 7, 2019

This is also super useful for sending stuff from Android to iOS, like photos. What else is needed to get this landed?

@cjbarth cjbarth mentioned this pull request Oct 7, 2019
@fzzzy
Copy link
Contributor

fzzzy commented Oct 7, 2019

The only thing required for this to land is for someone to have time to review it. There's nobody full time on send right now. I might be able to take a look at it next week.

@cjbarth
Copy link

cjbarth commented Oct 7, 2019

@oliver-dvorski It would be nice if there were no merge conflicts when this does get reviewed, hopefully next week. Would you be in a position to fix this merge conflict?

@oliver-dvorski
Copy link
Author

oliver-dvorski commented Oct 7, 2019 via email

@oliver-dvorski oliver-dvorski force-pushed the qr-code-download-link-proposal branch 2 times, most recently from 6a7676b to 7f110ba Compare October 11, 2019 19:37
@oliver-dvorski
Copy link
Author

Alrighty, I think I got myself out of that git mess I made. Should be squeaky clean now

@cjbarth
Copy link

cjbarth commented Oct 28, 2019

It looks like @dannycoates updated dependencies in this project and messed up the lock file again :( Maybe just leave the lock file out of this PR and let the maintainers add the lock for the dependencies that you need when they do their regular package updates?

@oliver-dvorski
Copy link
Author

@cjbarth Brilliant Idea! Unfortunately, the test pipeline is checking to see if the two files are in sync and when they're not - it throws an error. So I thought I'll just update the thing instead and just push whenever. Which is still cool, because now that I rebased on master, I see that there's a test that's failing so I can address that

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants