Add support for line breaks markdown and mathjax in metadata

[samuelcambien]

References

• Fixes Porting the MathJax display from previous DSpace versions #1404
• Fixes Preserve line breaks in metadata #1762
• Fixes Display line breaks in metadata #1763

Description

This PR focuses on two things:

• preserve and display line breaks in metadata values
• render markdown and mathjax in the dc.description.abstract metadata values

Instructions for Reviewers

List of changes in this PR:

• some new libraries have been added:
  • markdown-it
  • markdown-it-mathjax3
  • sanitize-html
• the class 'preserve-line-breaks' has been added to display line breaks for these components:
  • MetadataValuesComponent
  • TruncatablePartComponent
  • EditInPlaceFieldComponent
• two new configuration fields have been added:
  • enableMarkdown (boolean, default: false)
  • enableMathjax (boolean, default: false)
• new pipe 'MarkdownPipe' has been created, this will:
  • render markdown html for the input value
  • if the enableMathjax environment config is true:
    • lazy load the markdown-it-mathjax3 code
    • render mathjax html for the input value
  • because the default Angular sanitizer doesn't let svg tags through, and these are used by the mathjax html, the sanitize-html sanitizer is used here instead. It's configuration is tweaked to let the relevant tags through.
• new field 'enableMarkdown' has been added to the MetadataValuesComponent, this is controlled by the ItemPageFieldComponent, and is only true for the ItemPageAbstractFieldComponent
• the MetadataValuesComponent template has been updated to use the markdown pipe if both of the following conditions are true:
  • markdown is enabled for this metadata field
  • markdown is enabled in the environment config

To test this PR, create an item with line breaks, markdown and mathjax code (for instance this item), and verify that everything renders correctly.

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes TSLint validation using yarn run lint
• My PR doesn't introduce circular dependencies
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

[lgtm-com]

This pull request introduces 1 alert when merging eb4cda4 into edd193a - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[tdonohue]

@samuelcambien : This appears to have lint errors (try running yarn lint locally to see them).

Additionally, the size of this PR is massive (15K lines) because it is somehow attempting to replace the entire yarn.lock file. It's unclear why that is, when you've only added a few new dependencies?

Maybe you've somehow accidentally changed all the line endings in yarn.lock? If that's the case, it's possible we may want to add yarn.lock eol=lf to our .gitattributes to force it to always use unix line endings: https://github.com/DSpace/dspace-angular/blob/main/.gitattributes#L9-L16 (You could include this in this PR if you want, but keep in mind that adding this setting will likely mean you'll need to re-nomalize your local files, see the notes here: https://git-scm.com/docs/gitattributes#_end_of_line_conversion)

[lgtm-com]

This pull request introduces 1 alert when merging 2eb8243 into 001d6dc - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[lgtm-com]

This pull request introduces 1 alert when merging c744b3d into 001d6dc - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[samuelcambien]

Hi @tdonohue, I've fixed the issues you mentioned.

[tdonohue]

@samuelcambien : Apologies but this now has minor merge conflicts (likely caused by the recent merge of #1780). Could you rebase it when you get the chance?

[davide-negretti]

I suggest to add the markdown support in the search result too, if this requires a minimum effort

[davide-negretti]

I reviewed the code and it looks fine for me.

PS - I did not mean to close the PR, my mouse decided to make me click the wrong button :-D

[samuelcambien]

I suggest to add the markdown support in the search result too, if this requires a minimum effort

Hi @davide-negretti, great suggestion. Implementing this isn't very hard, but it looks a bit weird with the current truncatable-part components (see screenshot). An option would be to add a gradient to the bottom here, but this is currently out of scope.

[tdonohue]

@samuelcambien : It appears this PR is now failing unit tests? Could you see if those could be fixed?

[hardyoyo]

I'm testing this PR now, here are my results:

line breaks work correctly, with no additional configuration:

Markdown works correctly, when configured in config.dev.yml:

MathJax is a mixed bag, I copied the markup from the sample item on the demo server, as per the suggestion above. Some of it works, some of it does not. I suspect this might be a configuration issue?

[hardyoyo]

Regardless of the mixed results with MathJax, I'm still +1 on this, as-is (assuming we can resolve the test failure)

[tdonohue]

@samuelcambien : Overall, this is working better. But, in Production mode, I cannot load an Item page (where the Item has an abstract with mathjax & markdown) when this feature is disabled.

Here's what I tried:

• Create an Item with Markdown & MathJax in the abstract (your test item is perfectly good)
• Start in production mode (yarn build:prod; yarn serve:ssr)
• Turn this feature off. I.e. don't add the markdown section to your config.prod.yml
• Visit the Item you created. The abstract won't load & this error appears in the console:

ERROR TypeError: Cannot read properties of undefined (reading 'enabled')
    at MetadataValuesComponent_ng_container_2_Template (C:\Users\donohue\programming\dspace-angular\dist\server\main.js:1:1792568)

If I enable the feature, then it does work in Prod mode. The error only occurs when disabled.

This PR is also currently failing some specs, so that also needs to be addressed. Thanks!

[samuelcambien]

I've repaired the tests, the ItemPageFieldComponent test's config change was breaking the ItemPageAbstractFieldComponent test.
I've also repaired the production build, it was a mistake in the DefaultAppConfig.
Tested the production build both with and without markdown/mathjax.

[tdonohue]

👍 Thanks @samuelcambien ! The fixes work for me now. I've retested today in production mode and everything works well. I've tried the default settings (both disabled) and also enabling just markdown and both markdown & mathjax. Looks good now!

[alawvt]

Thank you very much, @samuelcambien and everyone, for working on this important improvement. Could someone add a screenshot of the test file with the configurations enabled, so can compare to it in our future testing? Thanks.

[tdonohue]

@alawvt : To enable both features in your config.*.yml, you'd use:

markdown:
  enabled: true
  mathjax: true

By default both are false.