Skip to content

Commit

Permalink
fix: native article nav issues (#8976)
Browse files Browse the repository at this point in the history
* fix: native article nav issues

* fix: remove title from webview tests

* chore: cleanup and tests
  • Loading branch information
gkartalis authored and MrSltun committed Jul 11, 2023
1 parent b809c8f commit f1f9468
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 7 deletions.
8 changes: 8 additions & 0 deletions src/app/Components/ArtsyWebView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,14 @@ export const ArtsyWebView = forwardRef<

const result = matchRoute(targetURL)

// TODO: need to implement the rest of native articles surfaces (news etc)
// the purpose of this is to prevent the webview from redirecting you again
// to the articles route, which would cause a loop and once in the webview to
// redirect you to either a native article view or an article webview
if (result.type === "match" && result.module === "Article") {
return
}

// if it's a route that we know we don't have a native view for, keep it in the webview
// only vanityURLs which do not have a native screen ends up in the webview. So also keep in webview for VanityUrls
// TODO:- Handle cases where a vanityURl lands in a webview and then webview url navigation state changes
Expand Down
12 changes: 8 additions & 4 deletions src/app/Scenes/Article/Components/ArticleWebViewScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,21 @@ export const ArticleWebViewScreen: React.FC<ArticleWebViewScreenProps> = ({ arti

return (
<Screen testID="ArticleWebViewScreen">
<Screen.Header title={data.title ?? ""} onBack={goBack} />
<Screen.Body>
<ArtsyWebView url={data.href} />
<Screen.Header onBack={goBack} />
<Screen.Body fullwidth>
{/*
NOTE: we don't need safeAreaEdges but passing undefined or empty array didn't work,
so we're passing "left" that doesn't actually add anything to the webview to avoid
having double paddings from Screen and ArtsyWebView
*/}
<ArtsyWebView url={data.href} safeAreaEdges={["left"]} />
</Screen.Body>
</Screen>
)
}

const articleWebViewFragment = graphql`
fragment ArticleWebViewScreen_article on Article {
title
href
}
`
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ describe("ArticleWebViewScreen", () => {
}),
})

expect(screen.getByText("Example Article")).toBeOnTheScreen()
expect(screen.UNSAFE_getByProps({ url: "/article/foo" })).toBeOnTheScreen()
})
})
1 change: 0 additions & 1 deletion src/app/Scenes/Article/__tests__/ArticleScreen.tests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ describe("ArticleScreen", () => {

await waitForElementToBeRemoved(() => screen.getByTestId("ArticleScreenPlaceholder"))

expect(screen.queryByText("Article Title")).toBeOnTheScreen()
expect(screen.UNSAFE_getByType(ArticleWebViewScreen)).toBeOnTheScreen()
})
})
1 change: 0 additions & 1 deletion src/app/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ function getDomainMap(): Record<string, RouteMatcher[] | null> {
addRoute("/works-for-you", "WorksForYou"),

// Webview routes
addWebViewRoute("/articles/:articleID"),
addWebViewRoute("/auction-faq"),
addWebViewRoute("/buy-now-feature-faq"),
addWebViewRoute("/buyer-guarantee"),
Expand Down

0 comments on commit f1f9468

Please sign in to comment.