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

Fix/multichain test webkit test failing #263

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

benmalcom
Copy link
Collaborator

The submit button on this line does not work on first click in webkit browsers, so the transaction does not even submit for the test to run. Until it double clicks.
The quick fix now is to double click the submit button if the browser is webkit, else traditional click.

Copy link

vercel bot commented Jun 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fast-auth-signer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2024 6:17pm

@benmalcom benmalcom changed the title Fix/multichain test browser rendering Fix/multichain test webkit test failing Jun 27, 2024
@benmalcom benmalcom marked this pull request as ready for review June 27, 2024 19:20
@@ -22,8 +22,11 @@ type TransactionDetail = {
class SignMultiChain {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is not the right solution, we can't expect the user to double click it.

I was investigating this and looks like the error comes from the forms validation, as you can see on the video attached. I'm checking how to fix it.

Idk why it only happens in Safari... very weird.

Screen.Recording.2024-07-19.at.14.58.18.mov

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did some changes on the forms and now it's passing locally with a single click (have to test on the CI yet)

The failing tests are the new ones I introduced. I'm working on it.

I will push the fix on my PR for the EVM function call.

Screenshot 2024-07-19 at 15 28 48

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey @Pessina thanks for spotting that, looking forward to your solution. Although the problem here is just on the local form we setup for testing e2e, that is, triggering the local form input click with playwright, we don't have such problem on real implementations like the demo app.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, agree, it's a problem only on the e2e app.

But it's better to fix the root problem instead of a quick fix.

I will open the PR soon, I'm struggling a bit with my tests now haha

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Haha okay then.
Do you want me to close this PR then?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Up to you.

Do you wanna check my solution first, to see if you are happy with it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All right then.
Let me wait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants