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

feat(share): add share component #335

Merged
merged 17 commits into from
Apr 22, 2020
Merged

feat(share): add share component #335

merged 17 commits into from
Apr 22, 2020

Conversation

KatvonRivia
Copy link
Member

No description provided.

/>
<h1 className={styles.title}>Share with the world</h1>
<div className={styles.shareButtons}>
<a href={'#'} target={'blank'} className={styles.button}>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what exactly should be shared on youtube or instagram.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also no idea! Youtube makes no sense and I have no idea how instagram works :)

@KatvonRivia KatvonRivia changed the title WIP feat(share): add share component feat(share): add share component Apr 17, 2020
@KatvonRivia KatvonRivia requested a review from pwambach April 17, 2020 14:00

export const CopyIcon: FunctionComponent = () => (
<svg
width="23"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All our icons should be 24x24 (if not 24px they should have at least quadratic dimensions)


export const TwitterIcon: FunctionComponent = () => (
<svg
width="27"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here: wrong size

import {ShareIcon} from '../icons/share-icon';
import config from '../../config/main';

import styles from './share.styl';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong import order

import config from '../../config/main';

import styles from './share.styl';
import {FormattedMessage} from 'react-intl';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong order

@@ -33,7 +33,11 @@ export default {
storyOfflinePackage: `https://storage.googleapis.com/esa-cfs-storage/${version}/stories/{id}/package.zip`,
storyMediaBase: `https://storage.googleapis.com/esa-cfs-storage/${version}/stories/{id}`,
stories: `https://storage.googleapis.com/esa-cfs-storage/${version}/stories/stories-{lang}.json`,
story: `https://storage.googleapis.com/esa-cfs-storage/${version}/stories/{id}/{id}-{lang}.json`
story: `https://storage.googleapis.com/esa-cfs-storage/${version}/stories/{id}/{id}-{lang}.json`,
facebook:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not api urls. Please create a separate section 'share' for them

onClick={() => setShowShare(true)}
/>
{showShare && (
<Overlay showCloseButton={false} onClose={() => {}}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is onClose an empty function? Better mark it optional and give it an empty function as its default value

className={styles.closeButton}
onClick={() => setShowShare(false)}
/>
<h1 className={styles.title}>Share with the world</h1>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing translations

/>
<h1 className={styles.title}>Share with the world</h1>
<div className={styles.shareButtons}>
<a href={'#'} target={'blank'} className={styles.button}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing underscore before _blank?

/>
<h1 className={styles.title}>Share with the world</h1>
<div className={styles.shareButtons}>
<a href={'#'} target={'blank'} className={styles.button}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also no idea! Youtube makes no sense and I have no idea how instagram works :)

<InstagramIcon />
<span>Instagram</span>
</a>
<button
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need some feedback here that the link has been copied.

@KatvonRivia KatvonRivia merged commit 9fa16a9 into develop Apr 22, 2020
@KatvonRivia KatvonRivia deleted the feat/share branch April 22, 2020 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants