Add DirectDebitField Component #464
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Abban
force-pushed
the
iban-accessibility
branch
8 times, most recently
from
c8c5937 to
f098189
Compare
Abban
force-pushed
the
iban-accessibility
branch
from
f098189 to
de1eb6d
Compare
Abban
force-pushed
the
iban-accessibility
branch
from
de1eb6d to
4f8c58c
Compare
Abban
force-pushed
the
iban-accessibility
branch
2 times, most recently
from
9790c1c to
92263b3
Compare
gbirke
approved these changes
Aug 29, 2024
There was a problem hiding this comment.
Thank you for splitting the commits, using very descriptive commit messages and having a well-structured explanation text in the PR. This has already helped me understand the ESLint changes and will prove very valuable in the future.
I'm approving this because my user tests did not reveal mistakes, it's more accessible and most of my comments are suggestions and questions. You can choose which ones you address and how.
Abban
force-pushed
the
iban-accessibility
branch
from
34aa809 to
471f0d4
Compare
This is drop in replacement for the PaymentBankData component that is more accessible and integrated with our form fields components. - Added accountNumber and bankCode fields, as the direct debit field will accept both IBAN and account numbers making the naming confusing. - Added getters actions and mutations for the new fields. - Updated the typescript interfaces to reflect the new structure. This makes the API calls that validate the bank data more consistent with our other API resources The pattern for our form fields is that they take in model values and do not access the store directly, We then use wrapper components to manage store access. That's what this component is for. This handles visibility of labels, and formatting values when fields receive input. As we're formatting the value in the input box we need a field similar to TextField that uses a prop as the value rather than take in a model. Ticket: https://phabricator.wikimedia.org/T364953
- Swap the PaymentBankData component for the new field - Remove old store injection with key - Update tests Ticket: https://phabricator.wikimedia.org/T364953
Now the donation and membership forms use the new components the old ones can be deleted. Ticket: https://phabricator.wikimedia.org/T364953
We use try catch blocks to handle rejected promises in async await functions. Eslint forces us to have the error variable but the typescript linter throws an error that they're unused.
Now the bank data values are all part of the store we can restore them on refresh. Ticket: https://phabricator.wikimedia.org/T364953
Abban
force-pushed
the
iban-accessibility
branch
from
471f0d4 to
cb47bf6
Compare
gbirke
approved these changes
Sep 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is drop in replacement for the PaymentBankData component
that is more accessible and integrated with our form fields
components.
Refactored the bankdata store
field will accept both IBAN and account numbers making the
naming confusing.
Added an API resource
This makes the API calls that validate the bank data more
consistent with our other API resources
Added a BankFields component
The pattern for our form fields is that they take in model
values and do not access the store directly, We then use
wrapper components to manage store access. That's what this
component is for.
Added a DirectDebitField
This handles visibility of labels, and formatting values
when fields receive input.
Added TextValueField
As we're formatting the value in the input box we need
a field similar to TextField that uses a prop as the value
rather than take in a model.
Ticket: https://phabricator.wikimedia.org/T364953