Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on May 22, 2021. It is now read-only.

Qr code download link proposal #1328

Open
oliver-dvorski wants to merge 9 commits into mozilla:master
base: master
Choose a base branch
Loading
from oliver-dvorski:qr-code-download-link-proposal

Conversation

@oliver-dvorski
Copy link

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

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 😄

hexandcube, june128, consatan, yvt, miguel-bluefox, cjbarth, jasonzhouu, thegoldgoat, mturchin, geomih, and 4 more reacted with thumbs up emoji cmpadden, akshaynexus, and mturchin reacted with laugh emoji
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.

Copy link
Author

@dannycoates Awesome 😊
Can't wait to refine this

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!

dbranham20 reacted with thumbs up emoji

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

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?

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 reacted with thumbs up emoji

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?

Copy link
Author

oliver-dvorski commented Oct 7, 2019 via email

Should be doable. Tomorrow I'll be heading back from vacation so I should have that fixed soon Chris Barth <notifications@github.com> schrieb am Mo., 7. Okt. 2019, 19:51:
...
@oliver-dvorski <https://github.com/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? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1328?email_source=notifications&email_token=ACE6RYITHXJOKF3BSL4LGJTQNNZJRA5CNFSM4HLLX2H2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEARHGLA#issuecomment-539128620>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACE6RYJIRW2VOGKY5VB7453QNNZJRANCNFSM4HLLX2HQ> .

@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
Copy link
Author

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

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?

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.

Reviewers

@flodolo flodolo Awaiting requested review from flodolo flodolo is a code owner

@johngruen johngruen Awaiting requested review from johngruen

1 more reviewer

@akshaynexus akshaynexus akshaynexus left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /