-
Notifications
You must be signed in to change notification settings - Fork 8
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 & viewer | Support for absolute link #1496
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tor and removed constructor
…hub.com/ni/nimble into users/vivin/revamp-rich-text-components
…omponent to model scripts Signed-off-by: Sai krishnan Perumal <[email protected]>
jattasNI
pushed a commit
that referenced
this pull request
Sep 12, 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. --> 1. To provide more spacing and facilitate the exploration of various functionalities of the rich text editor, setting a default height in storybook docs until enabling the `fit-to-content` attribute. 2. To make the content, such as the links (#1496 (comment)), visible if any are added as part of the test, height is added for storybook tests. ## 👩💻 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. --> 1. Add `height: 160px;` to the rich text editor storybook doc. 2. Add height for the theme matrix test to isolate the Chromatic test difference from the #1496 PR. ## 🧪 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. If any functionality is not covered by automated testing, provide justification. --> Verified manually the differences in the height of the component in storybook docs and tests. ## ✅ 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.
1 task
jattasNI
reviewed
Sep 12, 2023
packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts
Show resolved
Hide resolved
rajsite
reviewed
Sep 12, 2023
packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts
Show resolved
Hide resolved
packages/nimble-components/src/rich-text/models/markdown-serializer.ts
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text/models/markdown-parser.ts
Outdated
Show resolved
Hide resolved
8 tasks
jattasNI
approved these changes
Sep 13, 2023
packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts
Outdated
Show resolved
Hide resolved
rajsite
approved these changes
Sep 13, 2023
packages/nimble-components/src/rich-text/models/tests/markdown-serializer.spec.ts
Show resolved
Hide resolved
packages/nimble-components/src/rich-text/models/tests/markdown-serializer.spec.ts
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text/models/markdown-parser.ts
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text/models/markdown-serializer.ts
Outdated
Show resolved
Hide resolved
1 task
… into users/vivin/link-support
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
🤨 Rationale
<URL>
.nimble-anchor
. Part of Create a new anchor pattern to share styles across nimble components #1372<a>
as we have few constraints in renderingnimble-anchor
in the editor. Render "nimble-anchor" in rich text editor #1516👩💻 Implementation
Rich text editor:
openOnClick
linkOnPaste
, especially since hyperlinks are not supported for the initial pass.markdown-serializer.ts
.excludes: '_'
. This is because CommonMark's autolink does not support the simultaneous application of another formatting within the link, making it impossible to render a specific part of the link in autolink markdowns, like<https://**nimble**.ni.dev>
.Rich text viewer (or markdown-parser):
nimble-anchor
by updating the schema inmarkdown-parser.ts
.🧪 Testing
validate
in index.ts (only HTTP/HTTPS), assessing links within various nodes, and restricting the application of other marks (bold/italics) to the links within the editor.<a>
tags and not asnimble-anchor
within the editor. This distinction arises because we have adjusted the schema of the link in themarkdown-parser.ts
to parse it as animble-anchor
. It also coversgetMarkdown
is the same assetMarkdown
for links similar to the tests pattern we followed for other marks and nodes.autolink
,validateLink
, andnormalizeLinkText
in tokenizer rules and configurations set in themarkdown-parser.ts
.href
) in the<a>
tag is serialized to autolink markdown string.✅ Checklist