-
Notifications
You must be signed in to change notification settings - Fork 11
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
Make external links more obvious #543
Conversation
2611f01
to
afdf89b
Compare
6395660
to
0d0b8fc
Compare
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.
This will only impact links in the Back Matter. What about external links that may appear elsewhere such as in markup-multiline
typed fields?
e2bb385
to
1821678
Compare
src/components/OSCALMarkdownProse.js
Outdated
@@ -1,12 +1,23 @@ | |||
import React from "react"; | |||
import ReactMarkdown from "react-markdown"; | |||
import OpenInNewIcon from "@mui/icons-material/OpenInNew"; | |||
|
|||
const components = { |
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.
react-markdown
allows for you to change things that come from markdown.
https://github.com/remarkjs/react-markdown#appendix-b-components
We rewrite the <a>
tag to display the external link icon.
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 looks like there is also an external link for Categorization ID in OSCALSystemCharacteristics
To make the inevitable conflict a bit easier to deal with, tests will fail until #548 is in. It should be a simple change to get them to pass again. |
Using a css solution was not versatile enough to actually display the mui icon. Instead the `icon` tag will contain a OpenInNewIcon icon if the absolute url points to an external source.
6184c93
to
dd747f6
Compare
dd747f6
to
da656b6
Compare
src/components/OSCALMarkupProse.js
Outdated
/** | ||
* Renders a string of markdown to React elements | ||
* @param {String} props.text | ||
* @returns a React element from the markdown | ||
*/ | ||
|
||
export function OSCALMarkupMultiLine(props) { | ||
components.p = props.paragraphComponent ?? "p"; |
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.
This changes the base components
object in a way that I am not especially comfortable with.
<ReactMarkdown
components = {{
...components,
p: props.paragraphComponent ?? "p",
}}
>
{props.children}
</ReactMarkdown>
src/components/OSCALMarkupProse.js
Outdated
const components = { | ||
a: ({ ...props }) => | ||
props.href.startsWith("http") ? ( | ||
<a target="_blank" {...props}> | ||
<OpenInNewIcon /> {props.children} | ||
</a> | ||
) : ( | ||
<a {...props}>{props.children}</a> | ||
), | ||
}; |
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'd like to name this something a little more descriptive like baseComponents
or something like that.
src/components/OSCALMarkupProse.js
Outdated
const components = { | ||
a: ({ ...props }) => | ||
props.href.startsWith("http") ? ( | ||
<a target="_blank" {...props}> |
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.
Why <a>
and not <Link>
from MUI?
Closes #83