-
Notifications
You must be signed in to change notification settings - Fork 445
refactor support page to use accordion template #555
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
Conversation
|
The biggest change I'm seeing is that the word wrapping inside the accordions are different. This is not a criticism, and I'm unsure what we should do about it. If anything, it's more consistent in this newer version. As an example, here's the first question, which looks identical to me in this branch compared to what's live. The accordion expands to match the width of the window, though text wrapping does not. Here's a longer question For what's currently deployed, the goes to most of the width of the accordion element for this question, even though it doesn't do it for every question. Then the text wraps more tightly after the question like it does for most of our docs. For what's in this branch, the wrapping is more consistent except for text that's formatted very differently from the default. I don't have a strong opinion about this. I guess I like that this code is more consistent. I'm also okay with saying much of what's going on here is out of scope of this ticket. I'd like us to make that choice explicitly, though, instead of letting it go without saying. |
|
Good catch @h-m-m - I hadn't noticed this one since most do look identical. I personally think the tighter wrap makes it easier to read. But I'll let others chime in too. |
| ``` | ||
| </div> | ||
| {% include alert.html alert_class="usa-alert--warning" content="If there is no indication of a signature within the request URL or the request SAML, the request is not signed." %} | ||
| <h5>Check the signature algorithm</h5> |
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.
based on the screenshot that @h-m-m took, it looks like this code got pulled into a code block somehow? i don't think that's the intention of it, so can you please investigate this?
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.
Thanks for catching that! Should be fixed now.
Sgtpluck
left a comment
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.
i didn't check it locally, but the code lgtm



Refactor the /support page to use the accordion template for better maintainability and as a step towards https://gitlab.login.gov/lg-teams/FIE/team-fie-daily/-/issues/17