Allow sharing photos from the chat log #653
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
netpro2k
force-pushed
the
feature/photo-sharing
branch
from
e12003c to
6815a01
Compare
gfodor
reviewed
Nov 2, 2018
| @@ -89,9 +85,9 @@ export default class InviteDialog extends Component { | |||
| )} | |||
| {this.props.allowShare && | |||
| !navigator.share && ( | |||
| <a href={tweetLink} className={styles.linkButton} target="_blank" rel="noopener noreferrer"> | |||
| <button className={styles.linkButton} onClick={this.shareClicked.bind(this, shortLink)}> | |||
There was a problem hiding this comment.
I would have written this to be onClick=() => this.shareClicked(shortLink) but this is probably OK -- i find i don't use bind anymore due to the property syntax. (currying the second arg is interesting tho, very FP :))
gfodor
reviewed
Nov 2, 2018
src/react-components/presence-log.js
Outdated
| function SpawnPhotoMessage({ name, body: { src: url }, className, maySpawn, hubId }) { | ||
| let landingPageUrl = new URL(url); | ||
| const [hostname, port] = process.env.RETICULUM_SERVER.split(":"); | ||
| console.log(hostname, port, landingPageUrl.port); |
There was a problem hiding this comment.
I might consider rolling this routine into phoenix-utils.js, its where I've tried to centralize references to RETICULUM_SERVER
There was a problem hiding this comment.
any reason getReticulumFetchUrl(landingPageUrl.pathname.replace(".png", ".html")) wouldn't work? also might be worth future proofing this for other file extensions by just dropping the file extension instead of assumign .png
gfodor
reviewed
Nov 2, 2018
src/react-components/presence-log.js
Outdated
| body: PropTypes.object, | ||
| className: PropTypes.string, | ||
| hubId: PropTypes.string | ||
| }; |
There was a problem hiding this comment.
chat-message.js is a thing so maybe photo-message.js should be too
gfodor
reviewed
Nov 2, 2018
| @@ -0,0 +1,17 @@ | |||
| export function share(opts) { | |||
There was a problem hiding this comment.
Eh I figured there might be other share utils at some point
gfodor
reviewed
Nov 2, 2018
| @@ -0,0 +1,17 @@ | |||
| export function share(opts) { | |||
| if (navigator.share) { | |||
There was a problem hiding this comment.
I'd add a comment
// Use navigator.share otherwise just post to Twitter
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a share button to the "Foo took a photo" chatlog messages.
This uses
navigator.shareif available, otherwise falls back to twitter. I also reworked how we were sharing to twitter in other places to use twitter intents popup, which is a bit nicer style wise.todo: