-
Notifications
You must be signed in to change notification settings - Fork 6
PWA Share Target #388
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
PWA Share Target #388
Conversation
|
@rektide is attempting to deploy a commit to the Cosmik Team on Vercel. A member of the Team first needs to authorize it. |
| } | ||
| }, [props.initialUrl]); | ||
|
|
||
| const handleAddCard = (e: React.FormEvent) => { |
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.
this makes sure that if the same component gets recycled for multiple adds, that the new url will indeed take effect. non-zero chance this can be deleted, but i'm not set up to test how needed this was.
| onSuccess: () => { | ||
| setSelectedCollections([]); | ||
| props.onClose(); | ||
| window.history.replaceState({}, '', window.location.pathname); |
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.
explanation: clears the addUrl from the query string, so that the same url does not pre-fill itself again next time.
|
Woah thanks for this work! I'll review it sometime next week |
|
I'll have to update our developer docs to make it easier to setup a dev environment |
|
@rektide - didn't have time last week. aiming to review later this week! |
|
@rektide this is now merged to prod - can you test it out and see if it works as intended? |
|
@weswalla I wasn't having luck getting it working. Looks like Android has a WONTFIX on being able to use I just sent in #453, which should workaround this. Apologies for not handling Android appropriately the first time around. |
Resolves #242, huzzah, adding share_target support. Flow is:
/home?addUrl=<url>.ComponentDrawerpicks it up withuseSearchParamshook,AddCardDrawer.There's some extra
useEffect's sprinkled in to insure activation. But pretty simple. Left a comment on one that I'm not sure is really needed, but seems like a fair safeguard.This should supercharge my Semble usage; I'm super thrilled for this to land! Thanks https://seams.so for reminding me how great
share_targetis. I'd love forsembleto become a defaultShare...on my Android experience!Note: I don't actually have this app up and running locally. I've spent considerable time researching, coding, & checking work, but I can't promise this actually works. It likely wont hurt, though.
Note2: happy to improve this pr. Very bikeshed'y, but
addUrlfeels... ok. Not great. I kind of want better. Happy to go rebase this PR with a better name.