-
Notifications
You must be signed in to change notification settings - Fork 19
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
Display user feedback when IPFS fails to provide annotations #3396
Changes from 4 commits
3fb987d
5ed0618
8708350
ec98a50
5da33ac
2fd6e3d
639db85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
.main { | ||
padding: 20px 14px 20px 14px; | ||
width: 100%; | ||
position: relative; | ||
border-radius: var(--radius-normal); | ||
background-color: var(--colony-white); | ||
font-size: var(--size-smallish); | ||
font-weight: var(--weight-bold); | ||
} | ||
|
||
/* Theme */ | ||
.themePrimary { | ||
composes: main; | ||
background-color: var(--primary); | ||
color: var(--colony-white); | ||
} | ||
|
||
.themeInfo { | ||
composes: main; | ||
background-color: newInfoBlue; | ||
color: var(--colony-white); | ||
} | ||
|
||
.themeDanger { | ||
composes: main; | ||
background-color: var(--danger); | ||
color: var(--colony-white); | ||
} | ||
|
||
.themePinky { | ||
composes: main; | ||
background-color: color-mod(var(--pink) alpha(15%)); | ||
color: var(--dark); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export const main: string; | ||
export const themePrimary: string; | ||
export const themeInfo: string; | ||
export const themeDanger: string; | ||
export const themePinky: string; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
**Description** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very cool of you to add a styleguide entry for this, but I think we're going to discontinue this. (unless you guys have other opinions on this) Leave it in for now |
||
The CalloutCard component provides a standardsed way to display user information such as warnings. | ||
The CalloutCard's appearance can be customized through `theme`s. The content should be provided through the appropriate props. | ||
|
||
### Example | ||
|
||
Here's a super simple implementation of an CalloutCard, and different examples of customisation of the content: | ||
|
||
```jsx | ||
import CalloutCard from '~core/CalloutCard'; | ||
import Link from '~core/Link'; | ||
import ExternalLink from '~core/ExternalLink'; | ||
|
||
const MSG = defineMessages({ | ||
warningTitle: { | ||
id: 'dashboard.ActionsPageFeed.ActionsPageFeedItemWithIPFS.warningTitle', | ||
defaultMessage: `<span>Attention.</span> `, | ||
}, | ||
warningTextExternal: { | ||
id: 'dashboard.ActionsPageFeed.ActionsPageFeedItemWithIPFS.warningText', | ||
defaultMessage: `Unable to connect to IPFS gateway, annotations not loaded. <a>Reload to try again</a>`, | ||
}, | ||
warningTextInternal: { | ||
id: 'dashboard.ActionsPageFeed.ActionsPageFeedItemWithIPFS.internalLink', | ||
defaultMessage: `Unable to connect to IPFS gateway, annotations not loaded. {link}`, | ||
}, | ||
}); | ||
|
||
<CalloutCard | ||
label={...MSG.warningTitle} | ||
labelValues={{ | ||
span: (chunks) => ( | ||
<span className={styles.noIPFSLabel}>{chunks}</span> | ||
), | ||
}} | ||
description={...MSG.warningTextInternal} | ||
descriptionValues={{ | ||
link: ( | ||
<Link | ||
to={location.pathname} | ||
text={MSG.internalLink} | ||
className={styles.link} | ||
onClick={() => window.location.reload(false)} | ||
/> | ||
), | ||
}} | ||
/> | ||
|
||
<CalloutCard | ||
label={...MSG.warningTitle} | ||
labelValues={{ | ||
span: (chunks) => ( | ||
<span className={styles.noIPFSLabel}>{chunks}</span> | ||
), | ||
}} | ||
description={...MSG.warningTextExternal} | ||
descriptionValues={{ | ||
a: (chunks) => <ExternalLink href="/url">{chunks}</ExternalLink>, | ||
}} | ||
/> | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import React from 'react'; | ||
import { FormattedMessage, MessageDescriptor } from 'react-intl'; | ||
|
||
import { ComplexMessageValues } from '~types/index'; | ||
|
||
import { getMainClasses } from '~utils/css'; | ||
import styles from './CalloutCard.css'; | ||
|
||
const displayName = 'CalloutCard'; | ||
|
||
interface Appearance { | ||
theme?: 'primary' | 'info' | 'danger' | 'pinky'; | ||
} | ||
|
||
interface Props { | ||
/** Appearance object */ | ||
appearance?: Appearance; | ||
|
||
/** label text */ | ||
label: MessageDescriptor | string; | ||
|
||
/** Values for context text (react-intl interpolation) */ | ||
labelValues?: ComplexMessageValues; | ||
|
||
/** A string or a `messageDescriptor` that make up the cards's content */ | ||
description?: MessageDescriptor | string; | ||
|
||
/** Values for context text (react-intl interpolation) */ | ||
descriptionValues?: ComplexMessageValues; | ||
} | ||
|
||
const CalloutCard = ({ | ||
appearance = { theme: 'pinky' }, | ||
label, | ||
labelValues, | ||
description, | ||
descriptionValues, | ||
}: Props) => { | ||
return ( | ||
<div className={getMainClasses(appearance, styles)}> | ||
<FormattedMessage {...label} values={labelValues} /> | ||
<FormattedMessage {...description} values={descriptionValues} /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you should make them a |
||
</div> | ||
); | ||
}; | ||
|
||
CalloutCard.displayName = displayName; | ||
|
||
export default CalloutCard; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from './CalloutCard'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
.link { | ||
font-weight: var(--weight-bold); | ||
color: var(--sky-blue); | ||
} | ||
|
||
.link:hover { | ||
text-decoration: underline; | ||
} | ||
|
||
.noIPFSLabel { | ||
font-size: var(--size-smallish); | ||
font-weight: var(--weight-bold); | ||
color: var(--pink); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export const link: string; | ||
export const noIPFSLabel: string; | ||
export const noIpfsLabel: string; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,32 @@ | ||
import React, { useCallback } from 'react'; | ||
import { useLocation } from 'react-router-dom'; | ||
import { defineMessages } from 'react-intl'; | ||
|
||
import Comment, { Props as CommentProps } from '~core/Comment'; | ||
import CalloutCard from '~core/CalloutCard'; | ||
import Link from '~core/Link'; | ||
|
||
import { useDataFetcher } from '~utils/hooks'; | ||
import { ipfsDataFetcher } from '../../../../core/fetchers'; | ||
|
||
import styles from './ActionsPageFeedItemWithIPFS.css'; | ||
|
||
const displayName = 'dashboard.ActionsPageFeed.ActionsPageFeedItemWithIPFS'; | ||
|
||
const MSG = defineMessages({ | ||
warningTitle: { | ||
id: 'dashboard.ActionsPageFeed.ActionsPageFeedItemWithIPFS.warningTitle', | ||
defaultMessage: `<span>Attention.</span> `, | ||
}, | ||
warningText: { | ||
id: 'dashboard.ActionsPageFeed.ActionsPageFeedItemWithIPFS.internalLink', | ||
defaultMessage: `Unable to connect to IPFS gateway, annotations not loaded. {link}`, | ||
}, | ||
reloadLink: { | ||
id: 'dashboard.ActionsPageFeed.ActionsPageFeedItemWithIPFS.internalLink', | ||
defaultMessage: `Reload to try again`, | ||
}, | ||
}); | ||
interface Props extends CommentProps { | ||
hash: string; | ||
} | ||
|
@@ -24,7 +44,7 @@ const ActionsPageFeedItemWithIPFS = ({ | |
); | ||
|
||
const getAnnotationMessage = useCallback(() => { | ||
if (!annotation || !ipfsDataJSON) { | ||
if (!annotation) { | ||
return undefined; | ||
} | ||
const annotationObject = JSON.parse(ipfsDataJSON); | ||
|
@@ -34,8 +54,33 @@ const ActionsPageFeedItemWithIPFS = ({ | |
return undefined; | ||
}, [annotation, ipfsDataJSON]); | ||
|
||
const annotationMessage = getAnnotationMessage(); | ||
const location = useLocation(); | ||
// trouble connecting to IPFS | ||
if (!ipfsDataJSON) { | ||
return ( | ||
<CalloutCard | ||
label={MSG.warningTitle} | ||
labelValues={{ | ||
span: (chunks) => ( | ||
<span className={styles.noIPFSLabel}>{chunks}</span> | ||
), | ||
}} | ||
description={MSG.warningText} | ||
descriptionValues={{ | ||
link: ( | ||
<Link | ||
to={location.pathname} | ||
text={MSG.reloadLink} | ||
className={styles.link} | ||
onClick={() => window.location.reload(false)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @arrenv correct. Its superfluous - I will remove. |
||
/> | ||
), | ||
}} | ||
/> | ||
); | ||
} | ||
|
||
const annotationMessage = getAnnotationMessage(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add a spinner loader while trying to fetch / parse the IPFS hash (and subsequent object) |
||
/* | ||
* This means that the annotation object is in a format we don't recognize | ||
*/ | ||
|
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.
you can just do
padding: 20px 14px