Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LUPEYALPHA-615] FE journey handle ineligible as lack teaching #2947

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

asmega
Copy link
Contributor

@asmega asmega commented Jun 28, 2024

Context

Changes

  • Added new ineligible page
  • This is the first ineligible page for this journey so some plumbing was required to set everything up

@asmega asmega added the deploy Deploy a review app for this PR label Jun 28, 2024
@asmega asmega marked this pull request as ready for review June 28, 2024 15:59
Copy link
Contributor

@slorek slorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need a form object for the ineligible page.

@asmega
Copy link
Contributor Author

asmega commented Jun 30, 2024

I'm not sure we need a form object for the ineligible page.

The benefit of using a form object is it acts a point to hang stuff for things that happen in the view. At the moment theres not much there and I could delete the form object and just inline everything into the view. The downside though is that the next person that comes along is likely to stuff more things into the view than desired if more business logic comes along.

I think its a very low price to pay for the potential benefits down the line. Also it just makes the pattern of the other pages in the journey much the same.

@asmega asmega force-pushed the fe-ineligible-lack-teaching branch from 8bd08fd to ddd5697 Compare July 2, 2024 10:12
@asmega asmega force-pushed the fe-ineligible-lack-teaching branch from ddd5697 to ccddd65 Compare July 2, 2024 10:23
@asmega asmega merged commit edaee10 into master Jul 2, 2024
16 checks passed
@asmega asmega deleted the fe-ineligible-lack-teaching branch July 2, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Deploy a review app for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants