Create OSCALMarkdownProse Component #536

kperk103 · 2022-08-04T13:38:56Z

This will add a basic implementation of an OSCALMarkdownProse
component. Once the component has been implemented as desired, it will
be used to handle markdown in prose statements, metadata titles,
system implementation remarks, and control implementation descriptions.

Currently it is a draft and any suggestions are greatly appreciated.

Part of #156.

This will add a basic implementation of a possible OSCALMarkdownProse component.

tuckerzp · 2022-08-04T13:49:24Z

In order to get markdown to display at a basic level throughout the application, this seems like a decent start. @kylelaker & @brian-easyd I know there was some discussion outside the issue on what we want here. Is there anything that you'd think we would want to add for an initial implementation of dealing with markdown?

kylelaker · 2022-08-04T13:58:53Z

Great start! I think initially we want a test just to make sure that this does what we expect; so:

<OSCALMarkdownProse>*Hello*, **world**</OSCALMarkdownProse>

Should look like:

Hello, world

And that

const text = "<script>alert('Nasty script!')</script>"
<OSCALMarkdownProse children={text}/>

Doesn't do anything gross. In fact, I almost wonder if we should just pass skipHtml to the component?

src/components/OSCALMarkdownProse.js

This is first test to see if there markdown can be converted by matching it with its corresponding HTML Currently this test is failing due to a JEST configuration error between JS and TS. What would be the best way to resolve this? I've looked online and it changes the JEST configurations however I don't want to make any unecessary changes to the dependencies

schen2166 · 2022-08-04T15:05:17Z

This is a screenshot of the error I am getting with the React testing, as mentioned in the previous commit

kperk103 · 2022-08-04T15:12:09Z

This is a screenshot of the error I am getting with the React testing, as mentioned in the previous commit

We found this and tried the suggestions that were mentioned but couldn't get it working: https://stackoverflow.com/questions/49263429/jest-gives-an-error-syntaxerror-unexpected-token-export

brian-comply0 · 2022-08-04T15:26:42Z

I want to make sure everyone is aware of NIST's official description for exactly what should be handled by Markup-line and Markup-multiline:
https://pages.nist.gov/OSCAL/reference/datatypes/#markup-data-types

Ultimately our capabilities should do exactly this. No more. No less. For now I am OK with achieving a reasonable sub-set and expanding in a later sprint if this proves too big an effort for one sprint.

Also, as I mentioned yesterday, the stored data format must be markdown, but that does not automatically mean the editor must be markdown.

Ideally the user interface is friendly to security practitioners who are not likely to know markdown, yet can also accept markdown for those who know it.

Again, for now I'm good with one or the other (non-markdown friendly or markdown-friendly), provided any solution we select allows us to grow on top of the work we do now rather than having to replace it.

kylelaker · 2022-08-04T15:45:24Z

I want to make sure everyone is aware of NIST's official description for exactly what should be handled by Markup-line and Markup-multiline: https://pages.nist.gov/OSCAL/reference/datatypes/#markup-data-types

Ultimately our capabilities should do exactly this. No more. No less. For now I am OK with achieving a reasonable sub-set and expanding in a later sprint if this proves too big an effort for one sprint.

Hmm so that looks like another case of markdown with extensions since tables are supported. But then the single-line version has a more limited set... That'll take more time to actually fine-tune. And we have to decide if we want different behavior for viewing vs editing. We probably also need to our best to display whatever is there, even if it's not technically totally valid (someone put a table in a markup-line) but we shouldn't let a user write something like that.

Also, as I mentioned yesterday, the stored data format must be markdown, but that does not automatically mean the editor must be markdown.

We should be able to accomplish this by only using this new Component when viewing/displaying the text and not try to perform any translation of Markdown to HTML (for now).

Ideally the user interface is friendly to security practitioners who are not likely to know markdown, yet can also accept markdown for those who know it.

I'd like to handle this in a separate ticket (can we create one)? This will require finding/adding/integrating a WYSIWYG editor of some sort or using something like the text field editor here on GitHub. That will be a much larger lift.

Again, for now I'm good with one or the other (non-markdown friendly or markdown-friendly), provided any solution we select allows us to grow on top of the work we do now rather than having to replace it.

Hopefully we get that by starting to add a custom component (as we're doing here) that's very minimal and then adding more features over time. This was why I advocated for this approach rather than just using the <ReactMarkdown> component "raw" all over the place.

This adds a brief description of the OSCALMarkdownProse component and adds a configuration in package.json so that the component tests will be able to run

Made tests to see if the component can convert string literals to to its equivalent in HTML So far there are tests for - plaintext - Bold, Italics, Plaintext mixed, - Hyperlinks - Images

…mics/oscal-react-library into feature/handle-markdown

schen2166 · 2022-08-04T18:02:07Z

Currently the tests check whether something in markdown will convert to its HTML equivalent.
I'm wondering what other tests I should include in the OSCALMarkdownProse tests?
A new test will expect text in markdown to equal its equivalent in HTML

kylelaker · 2022-08-04T18:10:55Z

can we fix the linting?

kperk103 · 2022-08-04T18:20:46Z

src/components/OSCALMarkdownProse.test.js

@@ -0,0 +1,40 @@
+import React from "react";
+import { OSCALMarkdownProse } from "./OSCALMarkdownProse";


Suggested change

import { OSCALMarkdownProse } from "./OSCALMarkdownProse";

import OSCALMarkdownProse from "./OSCALMarkdownProse";

Right now the component tests are failing and I believe if you remove the brackets around OSCALMarkdownProse it will prevent them from failing

kylelaker · 2022-08-04T18:44:19Z

src/components/OSCALMarkdownProse.test.js

+import ReactDomServer from "react-dom/server";
+import OSCALMarkdownProse from "./OSCALMarkdownProse";
+
+export default function asHtml(input) {


Does this need to be exported? What happens if we don't do that?

Note: line does not allow multiple lines so not sure if this is what multiline markdown should be. I am unable to get embedded HTML tags like <h1> body <h2> header 2 </h2> more text </h1> I've also tried markdown with mutiple lines sich as `const multiline string = "line 1 \ line 2 \ line 3\"` and converting that to HTML and caused linting errors

kylelaker · 2022-08-04T19:34:21Z

src/components/OSCALMarkdownProse.test.js

+  const multilineMarkdown =
+    "this should b here # h1 Heading 8-    ## h2 Heading     This is in heading 2    ### h3 Heading      This is in heading 3    #### h4 Heading      ##### **h5 Heading**    ###### h6 Heading ";
+  const multilineAsHTML =
+    "<p>this should b here # h1 Heading 8-    ## h2 Heading     This is in heading 2    ### h3 Heading      This is in heading 3    #### h4 Heading      ##### <strong>h5 Heading</strong>    ###### h6 Heading</p>";


So uhh despite being called "multiline" this is just one line.

Suggested change

const multilineMarkdown =

"this should b here # h1 Heading 8- ## h2 Heading This is in heading 2 ### h3 Heading This is in heading 3 #### h4 Heading ##### **h5 Heading** ###### h6 Heading ";

const multilineAsHTML =

"<p>this should b here # h1 Heading 8- ## h2 Heading This is in heading 2 ### h3 Heading This is in heading 3 #### h4 Heading ##### <strong>h5 Heading</strong> ###### h6 Heading</p>";

const multilineMarkdown = `

this should be here

# h1 heading

## h2 Heading

This is in heading 2

### h3 Heading

This is in heading 3

##### **h5 Heading**

`;

const multilineHtml = "<p>this should be here</p><h1>h1 heading</h1><h2>h2 Heading</h2><p>This is in heading 2</p><h3>h3 Heading</h3><p>This is in heading 3 </p><h5><strong>h5 Heading</strong></h5>";

ya know +/- some formatting and spacing and all that

kylelaker

I am happy with this as a base component with simple tests to start the integration work with.

Bronstrom

This is a solid start to handle markdown throughout the application consistently. I also like seeing were creating our tests at the beginning of this issue. Great work!

naiqbal · 2022-08-12T15:24:56Z

@kylelaker @brian-easyd Demo of Issue #536 for acceptance https://easydynamics-my.sharepoint.com/:v:/p/niqbal/EVohd_Csn51OnQ09gFAH2osB65GDEKvCnjOI69rnAWNQ9A?e=ysfjUo

brian-comply0 · 2022-08-17T15:48:17Z

I agree this is complete

Add basic OSCALMarkdownProse component

0e8f6a1

This will add a basic implementation of a possible OSCALMarkdownProse component.

kperk103 requested review from kylelaker, brian-comply0 and tuckerzp August 4, 2022 13:43

schen2166 reviewed Aug 4, 2022

View reviewed changes

src/components/OSCALMarkdownProse.js Show resolved Hide resolved

Add a description for OSCALMarkdownProse and configuration

d4d08ab

This adds a brief description of the OSCALMarkdownProse component and adds a configuration in package.json so that the component tests will be able to run

kylelaker marked this pull request as ready for review August 4, 2022 16:13

easy-dynamics-oscal-automation bot requested a review from a team August 4, 2022 16:13

Merge branch 'develop' into feature/handle-markdown

7af5f41

kylelaker marked this pull request as draft August 4, 2022 16:14

schen2166 added 2 commits August 4, 2022 13:55

Preliminary Component tests for OSCALMarkdownProse

8f8d1a7

Made tests to see if the component can convert string literals to to its equivalent in HTML So far there are tests for - plaintext - Bold, Italics, Plaintext mixed, - Hyperlinks - Images

Merge branch 'feature/handle-markdown' of https://github.com/EasyDyna…

0151b82

…mics/oscal-react-library into feature/handle-markdown

Fix linting in component

6dc6b8c

kperk103 commented Aug 4, 2022

View reviewed changes

Fix linting errors on Prose Test

d47a7fd

kylelaker reviewed Aug 4, 2022

View reviewed changes

kylelaker suggested changes Aug 4, 2022

View reviewed changes

Fix markdown in multiple lines so that embedded HTML tags are returned

7bacf36

kperk103 marked this pull request as ready for review August 4, 2022 20:08

schen2166 linked an issue Aug 4, 2022 that may be closed by this pull request

Handle Markdown in Prose and Other Fields #156

Closed

kylelaker approved these changes Aug 4, 2022

View reviewed changes

easy-dynamics-oscal-automation bot requested a review from a team August 4, 2022 20:48

Bronstrom approved these changes Aug 4, 2022

View reviewed changes

kylelaker merged commit ae17d39 into develop Aug 4, 2022

kylelaker deleted the feature/handle-markdown branch August 4, 2022 23:07

naiqbal assigned kperk103 Aug 12, 2022

kylelaker added the enhancement New feature or request label Aug 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create OSCALMarkdownProse Component #536

Create OSCALMarkdownProse Component #536

kperk103 commented Aug 4, 2022 •

edited by tuckerzp

tuckerzp commented Aug 4, 2022

kylelaker commented Aug 4, 2022

schen2166 commented Aug 4, 2022

kperk103 commented Aug 4, 2022

brian-comply0 commented Aug 4, 2022

kylelaker commented Aug 4, 2022

schen2166 commented Aug 4, 2022

kylelaker commented Aug 4, 2022

kperk103 Aug 4, 2022

kylelaker Aug 4, 2022

kylelaker Aug 4, 2022 •

edited

kylelaker left a comment

Bronstrom left a comment

naiqbal commented Aug 12, 2022

brian-comply0 commented Aug 17, 2022

		@@ -0,0 +1,40 @@
		import React from "react";
		import { OSCALMarkdownProse } from "./OSCALMarkdownProse";

	import { OSCALMarkdownProse } from "./OSCALMarkdownProse";
	import OSCALMarkdownProse from "./OSCALMarkdownProse";

Create OSCALMarkdownProse Component #536

Create OSCALMarkdownProse Component #536

Conversation

kperk103 commented Aug 4, 2022 • edited by tuckerzp

tuckerzp commented Aug 4, 2022

kylelaker commented Aug 4, 2022

schen2166 commented Aug 4, 2022

kperk103 commented Aug 4, 2022

brian-comply0 commented Aug 4, 2022

kylelaker commented Aug 4, 2022

schen2166 commented Aug 4, 2022

kylelaker commented Aug 4, 2022

kperk103 Aug 4, 2022

Choose a reason for hiding this comment

kylelaker Aug 4, 2022

Choose a reason for hiding this comment

kylelaker Aug 4, 2022 • edited

Choose a reason for hiding this comment

kylelaker left a comment

Choose a reason for hiding this comment

Bronstrom left a comment

Choose a reason for hiding this comment

naiqbal commented Aug 12, 2022

brian-comply0 commented Aug 17, 2022

kperk103 commented Aug 4, 2022 •

edited by tuckerzp

kylelaker Aug 4, 2022 •

edited