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

Minor fixes related to line breaks & Markdown in metadata fields #1925

Merged
merged 3 commits into from
Oct 28, 2022

Conversation

ybnd
Copy link
Member

@ybnd ybnd commented Oct 21, 2022

References

Description

This PR bundles a few minor fixes related to #1851

  • Even spacing between comma-separated MDVs (e.g. subjects)
    20221021-180851

    This was an older issue, caused by line breaks within the <span>, which Angular translates into unwanted spacing. Now we include a space in the separator itself: ', '

  • Made enableMarkdown an input of ds-generic-item-page-field so it's easier to enable Markdown for arbitrary Item page fields

  • When the new dsMarkdown pipe is combined with the new .preserve-line-breaks CSS class, line breaks in Markdown are effectively doubled. Therefore, we shouldn't add this class when rendering Markdown.

Instructions for Reviewers

Confirm that this PR solves the issues listed above and that the changes are sensible.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • 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.

YanaDePauw and others added 3 commits October 18, 2022 12:11
The Markdown parser generates HTML with headings, paragraphs, ... for each bit of text.
This results in extra vertical spacing if we use `white-space: pre-line`
@ybnd ybnd marked this pull request as ready for review October 24, 2022 16:06
@ybnd
Copy link
Member Author

ybnd commented Oct 24, 2022

The e2e failures seem entirely unrelated to the changes here, so I'm hoping they're random.

@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label Oct 28, 2022
@tdonohue tdonohue self-requested a review October 28, 2022 20:05
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @ybnd ! I gave this a test today and markdown still works everywhere, and it looks better (with smaller line breaks). Code looks good too!

@tdonohue tdonohue added this to the 7.5 milestone Oct 28, 2022
@tdonohue tdonohue merged commit c876055 into DSpace:main Oct 28, 2022
4science-it pushed a commit to 4Science/dspace-angular that referenced this pull request Jul 8, 2024
[DSC-1708] popover width adjustment

Approved-by: Francesco Molinaro
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge bug improvement
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants