[ACQ-3328] WS Marketing Page - EnrollInForm - Update enrolInForm onClick to match what enroll on enroll page do #66569
Conversation
…d alerts and user info, make enroll actually work
…eWorkshopEnrollmentApi for improved API interaction
…ve button accessibility
|
Any naming suggestions are more then welcome 😄 |
| type="primary" | ||
| size="m" | ||
| href={buildEnrollButtonLink(`/professional-learning/workshops/${id}`)} | ||
| text="Sign-in to enroll" |
There was a problem hiding this comment.
Should this text be dependent on whether it's is_student or is_signed_out? i.e. should is_student say "Upgrade account to enroll" and is_signed_out say "Sign-in to enroll"?
There was a problem hiding this comment.
There was a problem hiding this comment.
I agree it will be nice to diverge messages for singned out and for student, but not sure about best naming. Would defer to @moshebaricdo on naming.
Maybe smth like Switch to teacher account OR Teacher Account needed? (feels to me like it gives more understanding/context)
There was a problem hiding this comment.
actually, also cc @bethanyaconnor @dmcavoy
There was a problem hiding this comment.
I think Moshe has a mock for the button for signed out. It says "Sign-in to enroll"
There was a problem hiding this comment.
For the student case I like "Switch to teacher account to enroll" @moshebaricdo what do you think?
There was a problem hiding this comment.
Let's do "Switch to teacher account," adding "to enroll" breaks onto a new line, but I think the overall context w/o it is clear enough.
| text: 'Sorry, we could not find this workshop. Please check the link and try again.', | ||
| }); | ||
| break; | ||
| case SUBMISSION_STATUSES.SUCCESS: |
There was a problem hiding this comment.
There's also a UNKNOWN_ERROR submission status that should be handled. That could potentially be the default in your switch/case/default setup here.
There was a problem hiding this comment.
Oh, I see thee confusion here. I took old enroll form as a base and there that status if handeled in api call, not in switch case. Totally agree it's better to handle that in switch case. Updating
| const submitEnrollment = async ( | ||
| params: WorkshopEnrollmentParams | null | ||
| ): Promise<WorkshopEnrollmentResponse | null> => { | ||
| setIsSubmitting(true); |
There was a problem hiding this comment.
Could there be a check that makes sure to return early if isSubmitting is already true to prevent a user from spam clicking the button and causing it to run multiple queries back to back, ensuring it just runs the query once until it finishes.
There was a problem hiding this comment.
Thank you for the catch!
There was a problem hiding this comment.
Also, I just realised that currently pending button is still clickable, and I think that's something we need to prevent (when isPending - functionally disable button)
|
Thanks for adding that alert @levadadenys! |
…ollment response logic
… attribute naming
2025-06-18.21.51.42.movUpdated naming for enroll button when signed in as student |
[ACQ-3328] WS Marketing Page - EnrollInForm - Update enrolInForm onClick to match what enroll on enroll page do
WS Marketing Page now supports WS enrollment.
Please note that editing enroll form values are not part of this ticket. All the values are taken from the user account.
Also updated text for signed out view of ws marketing page as I was working with that code anyways.
2025-06-17.17.07.10.mov
2025-06-17.17.06.04.mov
Links
ACQ-3328
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: