-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Refactor AppDetailsPage to Macaw Next #3818
Changes from 2 commits
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,5 @@ | ||
--- | ||
"saleor-dashboard": patch | ||
--- | ||
|
||
Refactored Manage App screen to use Macaw/next. Added missing empty-state messages, like missing permissions or app description. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,41 @@ | ||
import CardTitle from "@dashboard/components/CardTitle"; | ||
import Skeleton from "@dashboard/components/Skeleton"; | ||
import { Card, CardContent } from "@material-ui/core"; | ||
import { Box, BoxProps, Text } from "@saleor/macaw-ui/next"; | ||
import React from "react"; | ||
import { useIntl } from "react-intl"; | ||
import ReactMarkdown from "react-markdown"; | ||
|
||
import messages from "./messages"; | ||
|
||
interface AboutCardProps { | ||
type AboutCardProps = { | ||
aboutApp?: string | null; | ||
loading: boolean; | ||
} | ||
} & BoxProps; | ||
|
||
const AboutCard: React.FC<AboutCardProps> = ({ aboutApp, loading }) => { | ||
export const AboutCard: React.FC<AboutCardProps> = ({ | ||
aboutApp, | ||
loading, | ||
...boxProps | ||
}) => { | ||
const intl = useIntl(); | ||
|
||
const renderContent = () => { | ||
if (loading) { | ||
return <Skeleton />; | ||
} | ||
|
||
if (aboutApp) { | ||
return <ReactMarkdown source={aboutApp ?? ""} />; | ||
} else { | ||
return <Text>{intl.formatMessage(messages.noAboutApp)}</Text>; | ||
} | ||
}; | ||
|
||
return ( | ||
<Card> | ||
<CardTitle title={intl.formatMessage(messages.aboutAppTitle)} /> | ||
<CardContent> | ||
{!loading ? <ReactMarkdown source={aboutApp ?? ""} /> : <Skeleton />} | ||
</CardContent> | ||
</Card> | ||
<Box {...boxProps}> | ||
<Text variant={"heading"} as={"h2"} marginBottom={4}> | ||
{intl.formatMessage(messages.aboutAppTitle)} | ||
</Text> | ||
<Box>{renderContent()}</Box> | ||
</Box> | ||
); | ||
}; | ||
export default AboutCard; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,47 +1,51 @@ | ||||||||||||||||||||||||||||||||||||||||||||
// @ts-strict-ignore | ||||||||||||||||||||||||||||||||||||||||||||
import CardTitle from "@dashboard/components/CardTitle"; | ||||||||||||||||||||||||||||||||||||||||||||
import ExternalLink from "@dashboard/components/ExternalLink"; | ||||||||||||||||||||||||||||||||||||||||||||
import Skeleton from "@dashboard/components/Skeleton"; | ||||||||||||||||||||||||||||||||||||||||||||
import { Card, CardContent } from "@material-ui/core"; | ||||||||||||||||||||||||||||||||||||||||||||
import { Box, BoxProps, Text } from "@saleor/macaw-ui/next"; | ||||||||||||||||||||||||||||||||||||||||||||
import React from "react"; | ||||||||||||||||||||||||||||||||||||||||||||
import { FormattedMessage, useIntl } from "react-intl"; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
import messages from "./messages"; | ||||||||||||||||||||||||||||||||||||||||||||
import { useStyles } from "./styles"; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
interface DataPrivacyCardProps { | ||||||||||||||||||||||||||||||||||||||||||||
type DataPrivacyCardProps = { | ||||||||||||||||||||||||||||||||||||||||||||
dataPrivacyUrl?: string | null; | ||||||||||||||||||||||||||||||||||||||||||||
loading: boolean; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} & BoxProps; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
const DataPrivacyCard: React.FC<DataPrivacyCardProps> = ({ | ||||||||||||||||||||||||||||||||||||||||||||
export const DataPrivacyCard: React.FC<DataPrivacyCardProps> = ({ | ||||||||||||||||||||||||||||||||||||||||||||
dataPrivacyUrl, | ||||||||||||||||||||||||||||||||||||||||||||
loading, | ||||||||||||||||||||||||||||||||||||||||||||
...boxProps | ||||||||||||||||||||||||||||||||||||||||||||
}) => { | ||||||||||||||||||||||||||||||||||||||||||||
const classes = useStyles(); | ||||||||||||||||||||||||||||||||||||||||||||
const intl = useIntl(); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
if (!dataPrivacyUrl && !loading) { | ||||||||||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
const renderContent = () => { | ||||||||||||||||||||||||||||||||||||||||||||
if (loading) { | ||||||||||||||||||||||||||||||||||||||||||||
return <Skeleton />; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
if (dataPrivacyUrl) { | ||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||
<ExternalLink href={dataPrivacyUrl} target="_blank"> | ||||||||||||||||||||||||||||||||||||||||||||
<FormattedMessage {...messages.dataPrivacyDescription} /> | ||||||||||||||||||||||||||||||||||||||||||||
</ExternalLink> | ||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
return <Text>{intl.formatMessage(messages.noDataPrivacyPage)}</Text>; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
throw new Error("Leaking switch statement"); | ||||||||||||||||||||||||||||||||||||||||||||
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. Implementing return early is optional (non-blocking).
Suggested change
Was this error necessary? Seems strange 🤔 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. Yes. Component should render only for cases I want it to. If none of conditions are met, this is a bug in the code. Should not happen. Just like leaky switch/cases. Hence throwing error to quickly see that something wrong happened. If I return "null" here, we will never now if there is a bug 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. That's if/else with returning statements inside. Exception throw is unreachable 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. Fixed 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. suggestion: shouldn't it be logged into sentry? 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. Won't sentry automatically do that? Its unhandled on purpose 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 added error boundary and explicitely called Sentry |
||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||
<Card> | ||||||||||||||||||||||||||||||||||||||||||||
<CardTitle title={intl.formatMessage(messages.dataPrivacyTitle)} /> | ||||||||||||||||||||||||||||||||||||||||||||
<CardContent> | ||||||||||||||||||||||||||||||||||||||||||||
{!loading ? ( | ||||||||||||||||||||||||||||||||||||||||||||
<ExternalLink | ||||||||||||||||||||||||||||||||||||||||||||
className={classes.linkContainer} | ||||||||||||||||||||||||||||||||||||||||||||
href={dataPrivacyUrl} | ||||||||||||||||||||||||||||||||||||||||||||
target="_blank" | ||||||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||||||
<FormattedMessage {...messages.dataPrivacyDescription} /> | ||||||||||||||||||||||||||||||||||||||||||||
</ExternalLink> | ||||||||||||||||||||||||||||||||||||||||||||
) : ( | ||||||||||||||||||||||||||||||||||||||||||||
<Skeleton /> | ||||||||||||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||||||||||||
</CardContent> | ||||||||||||||||||||||||||||||||||||||||||||
</Card> | ||||||||||||||||||||||||||||||||||||||||||||
<Box {...boxProps}> | ||||||||||||||||||||||||||||||||||||||||||||
<Text variant={"heading"} marginBottom={4} as={"h2"}> | ||||||||||||||||||||||||||||||||||||||||||||
{intl.formatMessage(messages.dataPrivacyTitle)} | ||||||||||||||||||||||||||||||||||||||||||||
</Text> | ||||||||||||||||||||||||||||||||||||||||||||
<Box>{renderContent()}</Box> | ||||||||||||||||||||||||||||||||||||||||||||
</Box> | ||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
export default DataPrivacyCard; |
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.