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

Improve rendering of abstracts in email #775

Merged
merged 1 commit into from
Aug 27, 2021

Conversation

matt-bernhardt
Copy link
Member

@matt-bernhardt matt-bernhardt commented Aug 27, 2021

This updates how abstracts are rendered in the receipt email which is sent to students, in order to better respect the user input around line breaks and carriage returns.

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Quick note about a11y testing: I'm not aware of how to use ANDI or Wave on an email, but I did listen to the resulting email in my Outlook using Mac's VoiceOver. I may be missing something, but the contents of the email seemed to be read correctly. I'm not aware of any gotchas in email accessibility beyond this check.

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

@matt-bernhardt matt-bernhardt marked this pull request as draft August 27, 2021 18:16
@mitlib mitlib temporarily deployed to thesis-submit-pr-775 August 27, 2021 18:16 Inactive
@matt-bernhardt matt-bernhardt force-pushed the etd-380-abstract-breaks branch from cd7cbac to df367b3 Compare August 27, 2021 18:35
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-775 August 27, 2021 18:36 Inactive
@coveralls
Copy link

coveralls commented Aug 27, 2021

Coverage Status

Coverage remained the same at 94.839% when pulling 843ec96 on etd-380-abstract-breaks into 690cada on main.

@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-775 August 27, 2021 18:44 Inactive
** Why are these changes being introduced:

* Currently, users whose abstracts have line breaks / carriage returns
  are seeing them stripped out in the receipt email which they receive.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/etd-380

** How does this address that need:

* This renders the abstract field using the simple_format() function,
  which replaces carriage return characters \n and \r\n with <br> and
  <p> tags. This improves their rendering in the email template.

** Document any side effects to this change:

* The block of fields in the email template is now wrapped in a div tag
  rather than a p tag (to prevent nested p tags).
* Additionally, the receipt email now visually separates the abstract
  field from the rest of the submitted fields. I think this is fine,
  but have asked the project team on Slack to confirm (we can follow
  up in QA testing).
@matt-bernhardt matt-bernhardt force-pushed the etd-380-abstract-breaks branch from 4f8644f to 843ec96 Compare August 27, 2021 19:05
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-775 August 27, 2021 19:06 Inactive
@matt-bernhardt matt-bernhardt marked this pull request as ready for review August 27, 2021 19:22
@matt-bernhardt matt-bernhardt merged commit 22f65ee into main Aug 27, 2021
@matt-bernhardt matt-bernhardt deleted the etd-380-abstract-breaks branch August 27, 2021 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants