-
Notifications
You must be signed in to change notification settings - Fork 9
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
Refactor: enrollment success copy #997
Conversation
supports single or multi-paragraph details
{% if "logged-out" in page.classes %} | ||
<img class="success-image" src="{% static "img/icon/happybus.svg" %}" alt="{% translate "core.icons.happybus" context "image alt text" %}"> | ||
{% else %} | ||
<p>{% translate "enrollment.pages.success.p1" %}</p> | ||
<p>{% translate "enrollment.pages.success.p2" %}</p> | ||
<p>{% blocktranslate with help_link=help_link%}enrollment.pages.success.p3{{ help_link }}{% endblocktranslate %}</p> | ||
{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we still need this logged out view?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah thanks for this reminder, we talked about moving it out of the success view and I forgot to do that here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't actually make sense to move this out of the success view and create a specific "logged out" view - since the user can log out from various places in the app, we can't send them to a final "you are logged out" everytime they log out - we use the session.origin()
to know where to send the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 6f0db24
avoid rendering HTML when no media items
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this PR, after it is merged, the full Login.gov flow and signing out of it should be checked on dev-benefits.calitp.org/ to double-check the logged out page looks okay.
I am going to prep the data migrations before merging so I don't break dev. Will aslo post a working localhost migration that we can all use for testing against the Login sandbox + our local test server |
Closes #946
Closes #994 - I needed this to be able to add multiple paragraphs to the media list
Closes #1004 - added support for multiple paragraphs here anyway, it was an easy/small change to include
Before
After
MST CC verification
Login.gov verification
(note styling implementation is not finalized or addressed in this PR)