Skip to content
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

[Pairdrop] text send dialog prefill doesn't work #274

Open
3 tasks done
fm-sys opened this issue Mar 15, 2023 · 3 comments
Open
3 tasks done

[Pairdrop] text send dialog prefill doesn't work #274

fm-sys opened this issue Mar 15, 2023 · 3 comments
Labels
bug Something isn't working PairDrop speciffic

Comments

@fm-sys
Copy link
Owner

fm-sys commented Mar 15, 2023

Checklist

  • I have searched the existing issues for my problem. This is a new ticket, NOT a duplicate or related to another open issue.
  • I have updated "Snapdrop for Android" to the latest version. The bug is reproducible on this latest version.
  • I have checked, that this is NOT a general Snapdrop bug. It is only related to the Android app, and not reproducible with the website as well.

App version

master

Android version

10

Describe the bug

Text send prefill doesn't work, cause pairdrop uses a modified send text dialog

Steps to reproduce the bug

  • Activate floating text selection
  • select a text and choose "send with snapdrop"
  • -> Text should be prefilled, but it isn't

Stacktrace

No response

Screenshots and additional context

@schlagmichdoch maybe it's possible to rename the pairdrop.net html elements so that the current implementation works for both, snapdrop and pairdrop at the same time.

@fm-sys fm-sys added bug Something isn't working PairDrop speciffic labels Mar 15, 2023
@schlagmichdoch
Copy link
Contributor

Sorry for the delay, somehow I’ve overseen this.
Ueen had a similar problem with his addon and created a PR request. I closed it though and can only repeat what I said to him:

It took me a while to tidy up the code for PairDrop to follow the HTML style guide: https://google.github.io/styleguide/htmlcssguide.html#id_Attributes

To make it easier to develop I needed to tidy up a lot of code and I don‘t want to rollback just for compatibility.
Couldn’t you simply do a try catch to support both identifiers? Alternatively you could append the url via argument to the PairDrop url: https://pairdrop.net/?share-target=text&text=testtext or fire an event that starts the so called PasteMode. The UI then follows AirDrops example to send the marked text via one click. But you mentioned that you want the text to be editable so I would go with a try catch instead.

Alternatively, you could differentiate PairDrop and Snapdrop whenever the instance is changed and look at the html meta tag: <meta name="application-name" content="PairDrop">

@fm-sys
Copy link
Owner Author

fm-sys commented Mar 21, 2023

Yes sure, can be fixed on our side of course....

However I don't really like the UI of the PasteMode. It doesn't give any hit that content (and which type of content) is selected for sharing. The "Done" seems quite random, not as self-explanatory as it should IMHO.

Therefore I'll probably go with the JS injection approach.

Pairdrop How it is implement in Snapdrop for Android currently
Screenshot_20230321_150508_org mozilla firefox Screenshot_20230321_150956_com fmsys snapdrop

@schlagmichdoch
Copy link
Contributor

schlagmichdoch commented Mar 26, 2023

It doesn't give any hit that content (and which type of content)

Not quite true. It does differentiate between text and files and shows the number of files:

I like the idea of using a toast that is persistent though!

"abort" seemed not right to me as you can share the content with multiple receivers once you are in pasteMode (as it is with AirDrop). Therefore I decided for "Done". Maybe similar to your solution I could add a title "Share Text" / "Share Files" and move the "Done" button to the side. Do you have another suggestion?

As with AirDrop, I wanted the share to be executed by clicking only once. Having to approve the (prefilled) text box first, seems counter intuitive to me when I share text via the share-menu.

I could imagine a compromise though: Paste Mode stays as is is but when text is shared it says shared text ✎ and clicking it opens a dialog similar to the send text dialog where the text can be edited first.

What do you think?

@fm-sys
Copy link
Owner Author

fm-sys commented Mar 27, 2023

It doesn't give any hit that content (and which type of content)

Not quite true. It does differentiate between text and files and shows the number of files

Indeed. However I had totally overlooked this text, doesn't seem prominent enough to me.

"abort" seemed not right to me as you can share the content with multiple receivers once you are in pasteMode (as it is with AirDrop). Therefore I decided for "Done".

In english it is translated with "cancel" currently, another idea would be to use "finish". However "done" is fine as well, just lacking context currently.

Maybe similar to your solution I could add a title "Share Text" / "Share Files" and move the "Done" button to the side. Do you have another suggestion?

Sounds good. Or maybe order them vertically (although this would take some more screen space)

As with AirDrop, I wanted the share to be executed by clicking only once. Having to approve the (prefilled) text box first, seems counter intuitive to me when I share text via the share-menu.

I could imagine a compromise though: Paste Mode stays as is is but when text is shared it says shared text ✎ and clicking it opens a dialog similar to the send text dialog where the text can be edited first.

I think it should be fine even without the edit functionality. I primarily have implemented that way because I had no idea how to do it better.

@fm-sys fm-sys closed this as completed in ecfae98 Sep 8, 2023
@schlagmichdoch
Copy link
Contributor

However I don't really like the UI of the PasteMode. It doesn't give any hit that content (and which type of content) is selected for sharing. The "Done" seems quite random, not as self-explanatory as it should IMHO.

Tbh I did not like it that much either as it didn’t show prominently enough what was being shared and the „Done“ button blocked the button header which could be useful while sharing.

In the new version v1.10.0 I have reworked the now called share mode to resolve this. I have not tested it on your app though so this is a FYI:
IMG_5899
IMG_5900
IMG_5901

@fm-sys
Copy link
Owner Author

fm-sys commented Dec 14, 2023

The new UI indeed looks pretty cool and is way more prominent. Like it, thanks for the great work!

What are the endpoints for it / how would I trigger these components?

@fm-sys fm-sys reopened this Dec 14, 2023
Repository owner deleted a comment from schlagmichdoch Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PairDrop speciffic
Projects
None yet
Development

No branches or pull requests

2 participants