Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABCd129cfb43dc0: [Cashtab] Update configure.js with direct antd calls

Summary

This diff replaces all uses of Notification.js functions in Configure.js with direct calls to the antd notification component.

Integration tests added for key success and error notifications, however does not cover every single permutation otherwise it will clog the test suite. This is because the notification checks occur at the tail end of a very long end to end workflow.

Test Plan

npm test

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

emack created this revision.Feb 11 2024, 08:50

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 11 2024, 08:50

emack requested review of this revision.Feb 11 2024, 08:50

Harbormaster completed remote builds in B27063: Diff 45130.Feb 11 2024, 08:54

bytesofman requested changes to this revision.Feb 11 2024, 21:12

bytesofman added inline comments.

cashtab/src/components/Configure/__tests__/Configure.test.js
95–101 ↗	(On Diff #45130)	don't need this to be a variable if we are not using it again anywhere
142 ↗	(On Diff #45130)	ditto
205 ↗	(On Diff #45130)	ditto
245 ↗	(On Diff #45130)	ditto
299 ↗	(On Diff #45130)	ditto
323 ↗	(On Diff #45130)	ditto
415 ↗	(On Diff #45130)
480 ↗	(On Diff #45130)	ditto
512 ↗	(On Diff #45130)	ditto

This revision now requires changes to proceed.Feb 11 2024, 21:12

Updated var usage

Updated comments

Harbormaster completed remote builds in B27069: Diff 45141.Feb 12 2024, 06:40

Harbormaster completed remote builds in B27070: Diff 45142.Feb 12 2024, 06:45

Removed single reference var and comment

Harbormaster completed remote builds in B27071: Diff 45143.Feb 12 2024, 06:55

bytesofman requested changes to this revision.Feb 12 2024, 12:45

bytesofman added inline comments.

cashtab/src/components/Configure/Configure.js
891 ↗	(On Diff #45143)	see `data-testid` comments below
989 ↗	(On Diff #45143)	we should only use a `data-testid` if it is impossible or impractical to get this element by its contents In this case, we should use `getByText`. From the users perspective, we want to confirm that a notification appears that says `${manualContactAddress} added to Contact List` --- not that a notification with a given `data-testid` appears.
1036 ↗	(On Diff #45143)	ditto
1051 ↗	(On Diff #45143)	ditto