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

fix(prose): newline characters not displaying properly #717

Merged

Conversation

mpemy
Copy link
Contributor

@mpemy mpemy commented Mar 7, 2023

The MarkupMultine method is used instead of the Markuline in the getTextSegment method because this is where we handle prose part. However, this method add an empty line at the begining which does not align well with the placeholder label. This empty line should be remove.

Just address a typo that was mistakenly inserted in the last commit.

closes #619

…amics#619)

The MarkupMultine method is used instead of the Markuline in the getTextSegment method because this is where we handle prose part. However,
this method add an empty line at the begining which does not align well with the placeholder label. This empty line should be remove.
@CLAassistant
Copy link

CLAassistant commented Mar 7, 2023

CLA assistant check
All committers have signed the CLA.

@easy-dynamics-oscal-automation easy-dynamics-oscal-automation bot requested a review from a team March 7, 2023 17:26
@tuckerzp tuckerzp changed the title mpemy/display remove control elements fix(prose): newline characters not displaying properly Mar 7, 2023
@tuckerzp
Copy link
Contributor

tuckerzp commented Mar 7, 2023

I updated the title and body to better fit conventional commits.

Before committing, also be sure to run npm run lint:fix to fix linting issues. That should help you with passing the linting check.

mpemy and others added 6 commits March 8, 2023 10:23
 Your branch is up to date with 'fork/mpemy/display-remove-control-elements'.

 Changes to be committed:
        modified:   src/components/OSCALControlProse.js
Fixing the failing build due to linting issue.
The line of code that was causing this issue have been addressed.
There was method MarkupLines that was  added and then removed because it was not needed
I just forgot to delete it that is why lint was failing because I was importing a component that was not used and was inexistent.
I have simply deleted that reference.
…emy/oscal-react-library into mpemy/display-remove-control-elements

merging these two branches so that I can complete this commit and fix the failing build
@Bronstrom
Copy link
Contributor

Posting an up-to-date screenshot of SR-2.1 for convenience:

image

Copy link
Contributor

@kylelaker kylelaker left a comment

Choose a reason for hiding this comment

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

Can we just handle this in getTextSegment itself?

function getTextSegment(text, key) {
  if (!text) {
    return null;
  }
  return (
    <Typography component="span" key={key}>
      {
        text.includes("\n") ?
        <OSCALMarkupMultiline>{text}</OSCALMarkupMultiline> :
        <OSCALMarkupLine>{text}</OSCALMarkupLine>
      }
    </Typography>
  );
}

@mpemy
Copy link
Contributor Author

mpemy commented Mar 10, 2023

Thanks Kyle, I will do, I will handle that logic in getTextSegment.
Thanks for your suggestions.

Copy link
Contributor

@kylelaker kylelaker left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this!!

@tuckerzp tuckerzp merged commit 3c85e59 into EasyDynamics:develop Mar 10, 2023
@kylelaker kylelaker added the bug Something isn't working label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Control Prose not displaying newline characters properly
5 participants