feat(bankTransfer): support new countries and new fields #3543
Conversation
|
✅ Deploy Preview for adyen-web ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
size-limit report 📦
|
| expect(await screen.findByText('Branch code')).toBeInTheDocument(); | ||
| expect(await screen.findByText(mockResult.branchCode)).toBeInTheDocument(); |
There was a problem hiding this comment.
To test for accessibility better here we should find the value based on the label i.e
expect(screen.getByLabelText('Branch code').innerText).toBe(mockResult.branchCode)There was a problem hiding this comment.
You mean to make sure it's a semantic label? I'm going to test, but I think is a limitation of testing-library and it not considering that definition lists are labels, but my memory could be wrong. Honestly just copy and pasted from the code above.
There was a problem hiding this comment.
Yeah, it's not a label so what we do for example in PayTo.test.ts:L303 is:
const mandateAmount = screen.getByText('Amount').nextSibling;
expect(mandateAmount).toHaveTextContent('Up to A$40.01 per transaction');
There was a problem hiding this comment.
I think this also might be related: testing-library/dom-testing-library#140
There was a problem hiding this comment.
Two options I would like to propose. So I see that indeed, the bank instructions table component does not tie the label to the fields. This is actually not great accessibility wise.
Could we take this opportunity to either
- Refactor the markup to use a grid with each label actually using a label element with an id to link it to its value. One drawback here is that we might break backwards compatibility with the styles perhaps we can keep the classnames and still do this
- Simply just use
aria-labeledbyandidprops with the current markup. Then we can use thegetByLabelTexttests
What do you think?
m1aw
changed the title
|
Summary
This adds support for new
bankTransfervariants and also support for 2 new fieldsbankCodeandbranchCode.Tested scenarios
Fixed issue: