-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: add deploy page e2e tests #448
base: dev
Are you sure you want to change the base?
Conversation
test/cypress/e2e/data/mockDeploy.ts
Outdated
@@ -0,0 +1,10 @@ | |||
export const mockDeploy = { |
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.
Could you please update this to a function that returns this object? We should get in the habit of only assigning variables for mocks this way.
<TransactionGrid gap="2rem"> | ||
<DetailDataList gap="2rem"> | ||
{!!deploy?.amount && ( | ||
<div data-cy="transaction-details-card"> |
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.
This doesn't mess with any of the existing styling does it?
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.
No visible style changes. Only did this for testing purposes. Adding the data-cy id on InfoCardContentWrapper did not work.
<DetailDataLabel>{t('timestamp')}</DetailDataLabel> | ||
<TransactionDetailData data-testid="readable-time-stamp"> | ||
{withSkeletonLoading( | ||
<> | ||
{deploy?.status} | ||
<StatusIconWrapper>{statusIcon}</StatusIconWrapper> | ||
</>, | ||
deploy?.readableTimestamp, | ||
isLoading, | ||
{}, | ||
)} | ||
</DeployStatusData> | ||
</TransactionDetailData> |
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.
Do we not care about status on desktop?
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.
Both screens will get status. One is show on HideOnMobile and the other on HideOnDesktop
🔥 Summary