-
Notifications
You must be signed in to change notification settings - Fork 984
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
No ability to enter/paste assets exceeds the users's balance #18526 #18599
Conversation
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.
LGTM :)
Jenkins BuildsClick to see older builds (5)
|
[theme error] | ||
(assoc text-input-dimensions | ||
:color | ||
(if error |
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.
should be error?
??
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.
Done
@@ -100,60 +100,4 @@ | |||
(.then (fn [] | |||
(h/is-truthy (h/get-by-label-text :button-one)) | |||
(h/fire-event :press (h/get-by-label-text :button-one)) | |||
(h/was-called on-confirm)))))) |
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.
why are we deleting all these tests? 🤔
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.
Because the tests were covering the input of more than limit which were reset to the limit. Now, when entering more than limit, nothing should happen except for changing the style.
(h/fire-event :press (h/query-by-label-text :keyboard-key-5)) | ||
(h/wait-for #(h/get-by-text "$2850.00")))))) | ||
|
||
(h/test "Try to fill more than limit" |
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 test should still work but the check should be updated 👍 .
we can technically fill more than limit no? 🤔
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.
The same thing here
(h/fire-event :press (h/query-by-label-text :keyboard-key-5)) | ||
(h/wait-for #(h/get-by-text "$2850.00")))))) | ||
|
||
(h/test "Switch from crypto to fiat and check limit" |
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 seems like a valid test case still
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.
Again, when the input was more than the limit and a switch was happening, the input was changing to the limit. Now, nothing should happen except for the style changing
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.
Done
@@ -61,6 +61,12 @@ | |||
(>= (js/parseFloat balance) input-value))) | |||
(map first))) | |||
|
|||
(defn- reset-input-error | |||
[new-value prev-value input-error] | |||
(if (> new-value prev-value) |
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.
can be
(reset! input-error
(> new-value prev-value))
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.
Done
10% of end-end tests have passed
Failed tests (42)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (5)Click to expandClass TestCommunityOneDeviceMerged:
|
79% of end-end tests have passed
Failed tests (8)Click to expandClass TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (38)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
Hi @mmilad75 thank you for your work. No issues from my side. PR can be merged |
fixes #18526
Screen.Recording.2024-01-23.at.20.07.16.mov