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

Show a banner based on some url param #5620

Merged
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,6 @@ cypress/screenshots
reports*
junit.xml
cypress/videos

# Diagnostic reports (https://nodejs.org/api/report.html)
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
2 changes: 2 additions & 0 deletions src/desktop/components/react/stitch_components/NavBar.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react"
import styled from "styled-components"
import { NavBar as ReactionNavBar } from "v2/Components/NavBar"
import { FlashBanner } from "v2/Components/FlashBanner"
import { data as sd } from "sharify"

import { SystemContextProvider, SystemContextProps } from "v2/Artsy"
Expand Down Expand Up @@ -42,6 +43,7 @@ export const NavBar: React.FC<NavBarProps> = ({
<NavBarContainer id="main-layout-header">
{showStagingBanner && <StagingBanner />}
<ReactionNavBar />
<FlashBanner />
</NavBarContainer>
</SystemContextProvider>
)
Expand Down
13 changes: 13 additions & 0 deletions src/test/acceptance/home.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,17 @@ describe("Home page", () => {
const $ = await browser.page("/")
$.html().should.containEql("Browse Works for Sale")
})

it("renders the email confirmed banner when flash_message is confirmed", async () => {
const $ = await browser.page("/?flash_message=confirmed")

$("div[class*=Banner__Wrapper]").should.have.length(1)
$.html().should.containEql("Your email has been confirmed")
})

it("ignores flash_message that isn't explicitly supported", async () => {
const $ = await browser.page("/?flash_message=l33thaxor")

$("div[class*=Banner__Wrapper]").should.have.length(0)
})
})
22 changes: 17 additions & 5 deletions src/v2/Components/FlashBanner.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import React from "react"
import { Banner, Sans } from "@artsy/palette"

interface Props {
messageCode: FlashMessageKey
}
import qs from "qs"

export type FlashMessageKey =
/* email confirmed */
Expand All @@ -25,7 +22,22 @@ const messages: Record<FlashMessageKey, string | React.FC> = {
expired_token: "Link expired. Resend verification email.",
damassi marked this conversation as resolved.
Show resolved Hide resolved
}

export const FlashBanner: React.FunctionComponent<Props> = ({
export const FlashBanner: React.FunctionComponent = () => {
const [messageCode, setMessageCode] = React.useState(null)
const isClient = typeof window !== "undefined"

React.useEffect(() => {
if (isClient) {
erikdstock marked this conversation as resolved.
Show resolved Hide resolved
const query = qs.parse(window.location.search.slice(1))
query["flash_message"] && setMessageCode(query["flash_message"])
damassi marked this conversation as resolved.
Show resolved Hide resolved
}
}, [])

if (!messageCode) return null
Copy link
Member

Choose a reason for hiding this comment

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

Maybe move this above the effect to show that we're exiting early and to avoid registering the effect inside of react's execution loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I follow you here.

Copy link
Member

Choose a reason for hiding this comment

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

Meaning, if (!messageCode) return null can move up above the React.useEffect call. Since messageCode is null, and we know that via props, we can immediately check for it right at the top of the component and exit early.

return <FlashMessage messageCode={messageCode} />
}

export const FlashMessage: React.FC<{ messageCode: string }> = ({
messageCode,
}) => {
const Message = messages[messageCode]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React from "react"
import { storiesOf } from "storybook/storiesOf"
import { FlashBanner } from "v2/Components/FlashBanner"
import { FlashMessage } from "v2/Components/FlashBanner"

storiesOf("Components/FlashBanner", module).add(
"Successful Confirmation Message",
() => {
return <FlashBanner messageCode="confirmed" />
return <FlashMessage messageCode="confirmed" />
}
)
6 changes: 3 additions & 3 deletions src/v2/Components/__tests__/FlashBanner.jest.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { mount } from "enzyme"
import "jest-styled-components"
import React from "react"
import { FlashBanner } from "../FlashBanner"
import { FlashMessage } from "../FlashBanner"

describe("FlashBanner", () => {
it("renders a banner based on a message code", () => {
const wrapper = mount(<FlashBanner messageCode="confirmed" />)
const wrapper = mount(<FlashMessage messageCode="confirmed" />)
expect(wrapper.text()).toContain("Your email has been confirmed.")
})

it("returns nothing without complaint if the message code is not supported", () => {
const wrapper = mount(<FlashBanner messageCode={"bad_message" as any} />)
const wrapper = mount(<FlashMessage messageCode="bad_message" />)
expect(wrapper.html()).toBeNull()
})
})