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

Rich Text Editor | Add setMarkdown() and getMarkdown() methods to set and get content from Tip Tap editor #1424

Merged
merged 112 commits into from
Aug 22, 2023

Conversation

aagash-ni
Copy link
Contributor

@aagash-ni aagash-ni commented Aug 10, 2023

Pull Request

🤨 Rationale

This PR contains logic for parsing the assigned markdown content, setting it to the editor and also serializing the editor's data to markdown content.

AzDo Feature: https://dev.azure.com/ni/DevCentral/_backlogs/backlog/ASW%20SystemLink%20LIMS/Features/?workitem=2350963
Issue: #1288

👩‍💻 Implementation

  • Exposed getMarkdown() and setmarkdown() methods to facilitate the exchange of data with the editor.
  • Used MarkdownParser from prosemirror-markdown package for parsing markdown strings, DOM serializer from prosemirror-model package to serialize the content as DOM structure, XML Serializer to serialize it to HTML string and sets it to the editor.
  • To serialize the tip-tap document in getMarkdown() used MarkdownSerializer from prosemirror-markdown package to serialize the node based on schema.
  • Enabled emphasis and list rules in MarkdownParser, this allows users to set bold, italics, numbered, and bulleted lists in a CommonMark flavor to the editor. All other basic markdown formats are disabled.
  • Added only respective nodes and marks for bold, italics, numbered and bulleted lists in MarkdownSerializer

🧪 Testing

  • Added unit tests and visual tests for the component.
  • Manually tested and verified the functionality of the supported features.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

vivinkrishna-ni and others added 30 commits July 6, 2023 18:45
* @public
*/
public setMarkdown(markdown: string): void {
if (this.$fastController.isConnected) {
Copy link
Member

Choose a reason for hiding this comment

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

Re-opening again, unless I am mistaken I still do not see any test that asserts that the getMarkdown and setMarkdown methods actually work before the element is connected to the DOM. Every test seems to connect first and then test something.

I'll mark the PR approved assuming we have this test case and it goes well. @mollykreis or @m-akinc can one of y'all verify this test case is added (that the element can be created and calling set and get markdown functions even though the element is not attached to the dom, and after attached we see the expected content)

@m-akinc m-akinc self-requested a review August 21, 2023 16:10
@aagash-ni aagash-ni merged commit 42928b3 into main Aug 22, 2023
4 checks passed
@aagash-ni aagash-ni deleted the users/aagash/markdown-parser-serializer-editor branch August 22, 2023 12:26
vivinkrishna-ni added a commit that referenced this pull request Sep 6, 2023
# Pull Request

## 🤨 Rationale

<!---
Provide some background and a description of your work.
What problem does this change solve?

Include links to issues, work items, or other discussions.
-->
Reorganize the folder structure of the `nimble-rich-text-editor` and
`nimble-rich-text-viewer` to share the common functionalities between
components.

Part of #1447

## 👩‍💻 Implementation

<!---
Describe how the change addresses the problem. Consider factors such as
complexity, alternative solutions, performance impact, etc.

Consider listing files with important changes or comment on them
directly in the pull request.
-->

Revamped folder structure in nimble-components:
```
/rich-text
    /editor
        editor component files...
    /viewer
        viewer component files...
    /specs
        README.md (common spec file for both editor and viewer)
```

Revamped folder structure in nimble-angular:
```
/rich-text
    /editor
        editor component files...
    /viewer
        viewer component files...
```

Related discussion:
#1424 (comment)

## 🧪 Testing

<!---
Detail the testing done to ensure this submission meets requirements. 

Include automated/manual test additions or modifications, testing done
on a local build, private CI run results, and additional testing not
covered by automatic pull request validation.
-->
- Manually tested the components in the local storybook build.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
jattasNI added a commit that referenced this pull request Sep 8, 2023
…ls (#1492)

# Pull Request

## 🤨 Rationale

- Created models for markdown functionalities to be used in rich text
editor and text viewer components.

## 👩‍💻 Implementation

- Moved markdown parsing and serializing functionalities from the rich
text editor and viewer components into respective models.
- `markdown-parser.ts` - Initialize the Prose Mirror markdown parse and
expose the `parseMarkdownToDOM` method for parsing the markdown string
into a corresponding document fragment, which can then be utilized by
both the rich text components to load it in the UI.
- `markdown-serializer.ts` - Initialize the Prose Mirror markdown parse
and expose the `serializeDOMToMarkdown` method to convert the rendered
node in the editor to a markdown string.

Related discussion:
#1424 (comment)

## 🧪 Testing

- Migrated all the markdown parsing unit test cases from the components
into `markdown-parser.spec.ts`

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Signed-off-by: Sai krishnan Perumal <[email protected]>
Signed-off-by: Sai krishnan Perumal <[email protected]>
Co-authored-by: SOLITONTECH\vivin.krishna <[email protected]>
Co-authored-by: Jesse Attas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants