va-button & va-button-pair: allow custom text for continue variation #1485
Conversation
| alert ("form onsubmit method fired!--on form"); | ||
| handleSubmit : (event)=>{ | ||
| console.log("Form onSubmit callback fired!", event); | ||
| alert ("Form onSubmit callback fired!"); |
There was a problem hiding this comment.
Just some cleanup/clarification of the Storybook page. This is not related to the custom text update.
| if (this.continue) return 'Continue'; | ||
| if (this.back) return 'Back'; | ||
| if (this.loading && !this.text) return 'Loading...'; | ||
| return this.text; | ||
| return; |
There was a problem hiding this comment.
Could this lead to a situation where, if text is undefined, the button renders without text?
Also, by having the custom text take precedence, this appears to result in the "Loading..." text being unreachable. This means that devs would need to manually watch for and update the custom text prop themselves if they wanted it to represent a loading state, yes?
I don't know if these things need to change, just thinking out loud as to whether they're issues or not.
There was a problem hiding this comment.
Could this lead to a situation where, if text is undefined, the button renders without text?
This is what I wanted to test with a pre-release in vets-website to see if any e2e tests failed. 😄 Technically right now, you could not set this.text and there would be no default text unless a variation was also set (back or continue). So I don't think anything is changing with what it would already do now. I thought a pre-release could absolutely confirm though and I'm still mulling that over just for our sanity.
Also, by having the custom text take precedence, this appears to result in the "Loading..." text being unreachable. This means that devs would need to manually watch for and update the custom text prop
Yes, that's a good call-out and what you're describing is exactly what would need to be done. Here is the existing example in Storybook:
I wonder if that could be refactored to be less manual for devs? Maybe as a follow-up? 🤔
Chromatic
https://3724-button-continue--65a6e2ed2314f7b8f98609d8.chromatic.com
Description
This will allow for a button text override when the
continuevariation is being used forva-buttonandva-button-pair. The default text will still be "Continue".Notes from DSC meeting about opening up button component text: department-of-veterans-affairs/vets-design-system-documentation#2362 (comment)
Closes department-of-veterans-affairs/vets-design-system-documentation#3724
QA Checklist
Screenshots
Acceptance criteria
Definition of done