feat: zap page #1566
feat: zap page #1566
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Playwright test results
Details
Skipped testschromium › e2e.spec.ts › Jumper full e2e flow › Should be able to navigate to profile and open first Mission |
Test results (4/4)
Details
|
Test results (3/4)
Details
Failed testschromium › swapActions.spec.ts › On chain swaps › Check ETH to USDTUSDT swap pair on ETH chain |
Test results (1/4)
Details
Skipped testschromium › e2e.spec.ts › Jumper full e2e flow › Should be able to navigate to profile and open first Mission |
Test results (2/4)
Details
Skipped testschromium › e2e.spec.ts › Jumper full e2e flow › Should be able to open quests mission page and switch background color |
There was a problem hiding this comment.
@oktapodia @tcheee This is a copy of the "Widget.tsx" component -> The only change is that we do not want the box-shadow within the widget-theme. First, I started out to create a partner theme to be used for the "zap" layout but ran into the problem that partner-themes do not support multiple themeModes (light + dark).
There was a problem hiding this comment.
Would be much better to refactorize the Widget.tsx instead of fully duplicating it :(
There was a problem hiding this comment.
Good idea running that extra mile instead of taking the shortcut I did!
I added a useWidgetConfig-hook -> We had such in the past as well, it now also takes an optional "customWidgetTheme" prop and is merged with the "source" widgetTheme using deepmerge by mui -> Wdyt?
src/app/[lng]/zap/[slug]/page.tsx
Outdated
| title: 'Jumper | Berachain', | ||
| description: 'Dive into the Berachain universe!', | ||
| siteName: siteName, | ||
| url: `${getSiteUrl()}/berachain/explore/${params.slug}`, |
There was a problem hiding this comment.
Are you sure about this url? it seems to be /zap/[slug]
There was a problem hiding this comment.
@tcheee Can we align on the meta information?
src/app/[lng]/zap/[slug]/page.tsx
Outdated
| }; | ||
| } catch (err) { | ||
| return { | ||
| title: `Jumper Learn | ${sliceStrToXChar(params.slug.replaceAll('-', ' '), 45)}`, |
There was a problem hiding this comment.
Not sure this is a learn page
| export default async function Page({ params }: { params: { slug: string } }) { | ||
| const { data } = await getQuestBySlug(params.slug); | ||
| const questData = (data.data[0] as ExtendedQuest) || undefined; | ||
|
|
There was a problem hiding this comment.
Do not forget to return notFound() if it does not exists
src/app/[lng]/zap/[slug]/page.tsx
Outdated
|
|
||
| export default async function Page({ params }: { params: { slug: string } }) { | ||
| const { data } = await getQuestBySlug(params.slug); | ||
| const questData = (data.data[0] as ExtendedQuest) || undefined; |
There was a problem hiding this comment.
Would be better to type is within getQuestBySlug
There was a problem hiding this comment.
Done, please have another look.
| marginTop: `calc( ${DEFAULT_WIDGET_TOP_OFFSET_VAR} - ${HeaderHeight.MD}px - ${DEFAULT_WIDGET_TOP_HOVER_OFFSET}px )`, | ||
|
|
||
| // positioning of widget on mobile-screens from 700px height | ||
| [`@media screen and (min-height: 700px)`]: { |
There was a problem hiding this comment.
Would be good to use breakpoints value instead to avoid regresions
There was a problem hiding this comment.
This is magic voodoo happening. Please note, it´s applying styles to the min-height which we usually don´t have as default breakpoints. It´s needed to position the widget accordingly while the welcomeScreen is open.
There was a problem hiding this comment.
Would be much better to refactorize the Widget.tsx instead of fully duplicating it :(
…o-use-the-zap-widget
…o-use-the-zap-widget
|
|
||
| export default async function Page({ params }: { params: { slug: string } }) { | ||
| const { data, url } = await getQuestBySlug(params.slug); | ||
|
|
||
| if (!data?.data?.[0]) { | ||
| if (!data) { |
There was a problem hiding this comment.
Might not be enough, data can be returned and data.data -> [] but will still not go through
There was a problem hiding this comment.
While adding the proper types on our fetch function getQuestBySlug I have also tweaked the return statement to the following (see screen). If there is no returned data, it should actually be undefined -->
| @@ -44,10 +44,8 @@ import { notFound } from 'next/navigation'; | |||
|
|
|||
| export default async function Page({ params }: { params: { slug: string } }) { | |||
| const { data, url } = await getQuestBySlug(params.slug); | |||
|
|
|||
| if (!data?.data?.[0]) { | |||
| if (!data) { | |||
There was a problem hiding this comment.
Yes, I have updated this everywhere where getQuestBySlug is being called. Check my answer above.
| const questData = data; | ||
|
|
||
| if (questData) { | ||
| const protocolDetails = zapMarkets.filter( |
There was a problem hiding this comment.
Dangerous, if find does not find any protocolDetails, it will be null, [0] will crash the app
There was a problem hiding this comment.
Are you sure? Please take a look on my updated getQuestBySlug function. With that, we should not run into that issue, but please let´s double-check.
src/app/lib/getQuestBySlug.ts
Outdated
| @@ -18,5 +33,12 @@ export async function getQuestBySlug(slug: string) { | |||
|
|
|||
| const data = await res.json(); // Extract data from the response | |||
|
|
|||
| return { data, url: apiBaseUrl }; // Return a plain object | |||
| if (!data || !Array.isArray(data.data) || data.data.length === 0) { | |||
There was a problem hiding this comment.
This part seems weird and does not seem to achieve anything, let's have a talk about it
There was a problem hiding this comment.
Ahhh, yes this is the part I was so confident about - that this could catch any errors --> I have added another check and hopefully catch all possible red-flags.
if (
!data || // Check if data is undefined or null
!data.data || // Check if data.data is undefined or null
!Array.isArray(data.data) || // Check if data.data is not an array
data.data.length === 0 || // Check if data.data is an empty array
data.data[0] === 0 // Check if the first element is exactly zero (assuming this is an invalid value)
) {
return { data: undefined, url: apiBaseUrl };
}
Not sure if that´s the proper way of handling errors but it did fine on all my test cases:
❌ data = null
❌ data = undefined
❌ data = []
❌ data = [0]
❌ data = { data : undefined }
❌ data = { data: 0 }
❌ data = { data: [] }
❌ data = { data: [0] }
✅ data = { data: [1] }
src/hooks/useWidgetConfig.ts
Outdated
| ...publicRPCList, | ||
| }; | ||
| } catch (e) { | ||
| if (process.env.DEV) { |
There was a problem hiding this comment.
Dangerous there, DEV is not a standard env variable, error will be obfuscated all time and not throwing anything (making it harder to debug), just remove this if
There was a problem hiding this comment.
@oktapodia Can you confirm and take another look if that is what you had in mind?
…o-use-the-zap-widget
…o-use-the-zap-widget
…o-use-the-zap-widget
Ticket: https://lifi.atlassian.net/browse/LF-11136