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

Create a new anchor pattern to share styles across nimble components #1372

Open
Tracked by #1288
vivinkrishna-ni opened this issue Jul 21, 2023 · 4 comments
Open
Tracked by #1288
Labels
enhancement New feature or request

Comments

@vivinkrishna-ni
Copy link
Contributor

vivinkrishna-ni commented Jul 21, 2023

🧹 Tech Debt

Rationale: #1335 (comment)

There are instances where we aim to maintain uniform anchor styles across various Nimble components, especially when using links similar to the one mentioned in the above comment. This is particularly relevant in situations where incorporating the actual nimble-anchor becomes challenging. Therefore, we can transfer the default anchor styles to the patterns and apply them consistently across components as required.

@jattasNI
Copy link
Contributor

Instead of sharing styles, another option we should explore is to emit nimble-anchor in the HTML rather than a. This would be even better for keeping the behaviors in sync as we add new non-style-related features to the anchor. (This would only affect the Nimble views; the markdown would still store the link using the normal [text](url) or http://inline-link.com syntax so other places that render the markdown like emails wouldn't necessarily need nimble anchors).

I took a very quick look at the tiptap link extension source and didn't see any support for emitting an arbitrary tag, but it looks like it might be really easy to add a configuration. The relevant source shows just two functions that emit and parse a tags. If those two functions could be changed to use a tag that was supplied as configuration, it could solve our use case.

I would advocate for us to try submitting an issue and/or PR to TipTap that proposes that enhancement to the extension. If they welcome the contribution then that direction seems preferable to the approach of only sharing styles.

@rajsite
Copy link
Member

rajsite commented Jul 31, 2023

Not sure how the features are related but the prose mirror marks docs also discuss how to customize the html of marks of which links are one type: https://prosemirror.net/examples/schema/#:~:text=Links

@vivinkrishna-ni
Copy link
Contributor Author

Thanks for sharing the references. We could possibly use nimble-anchor in both nimble-rich-text-editor and nimble-rich-text-viewer with the above TipTap configurations and updating the Prosemirror schema. It is incorporated in this PR: #1496

However, the nimble-anchor in the editor is clickable by default whereas the a tag default behavior inside contenteditable div is non-clickable. For more details, refer to this issue: #1502

@vivinkrishna-ni
Copy link
Contributor Author

vivinkrishna-ni commented Sep 12, 2023

In addition to the above-linked issue, we have captured a few more nimble-anchor behavior in nimble-rich-text-editor at the first pass of development that has to be fixed before rendering it in the editor component. Here is the tech debt issue created for the same: #1516.

But we could use the nimble-anchor in rich-text-viewer as it is not an editable component.

vivinkrishna-ni added a commit that referenced this issue Sep 14, 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. Add absolute links in the editor by either pasting or manually
entering a URL (HTTP/HTTPS).
2. The input and output for absolute links in Markdown follow the
[AutoLink ](https://spec.commonmark.org/0.30/#autolink) format in the
CommonMark flavor i.e. `<URL>`.
3. Links in the viewer will render as `nimble-anchor`. Part of
#1372
4. Links in the editor will render as `<a>` as we have few constraints
in rendering `nimble-anchor` in the editor.
#1516

## 👩‍💻 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.
-->

#### Rich text editor:
1. Installed [@tiptap/extension-link](https://tiptap.dev/api/marks/link)
to enable link in editor.
2. Configuring the following settings for the link in the editor:
- Preventing users from opening a link in the editor by disabling the
[`openOnClick`](https://tiptap.dev/api/marks/link#open-on-click)
- Disallowing users from pasting a link to an already selected word by
disabling
[`linkOnPaste`](https://tiptap.dev/api/marks/link#link-on-paste),
especially since hyperlinks are not supported for the initial pass.
- Add Regex to [validate](https://tiptap.dev/api/marks/link#validate)
that the entered/parsed URL conforms to the HTTPS/HTTP protocol. URLs
not using HTTPS/HTTP will be displayed as plain text.
5. Adding custom link serializer for autolink in
`markdown-serializer.ts`.
6. Preventing the application of additional formatting, such as bold or
italics, to a link by using [`excludes:
'_'`](https://prosemirror.net/docs/ref/#model.MarkSpec.excludes). This
is because CommonMark's
[autolink](https://spec.commonmark.org/0.30/#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):
1. Rendering links as `nimble-anchor` by updating the schema in
`markdown-parser.ts`.
2.
[Validate](https://markdown-it.github.io/markdown-it/#MarkdownIt.prototype.validateLink)
if the links in the input markdown string are HTTPS/HTTP.
3. [Normalize the link
text](https://markdown-it.github.io/markdown-it/#MarkdownIt.prototype.normalizeLinkText)
to render the link as is, instead of updating the link text if it
contains any encoded or non-ASCII characters.

## 🧪 Testing
1. rich-text-editor.spec.ts:
1. "Absolute link interactions in the editor" - These tests cover user
interactions with the editor like typing or copying and pasting the
links. This includes testing the `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.
2. "Absolute link markdown tests" - These tests ensure the parsed links
are rendered as `<a>` tags and not as `nimble-anchor` within the editor.
This distinction arises because we have adjusted the schema of the link
in the `markdown-parser.ts` to parse it as a `nimble-anchor`. It also
covers `getMarkdown` is the same as `setMarkdown` for links similar to
the tests pattern we followed for other marks and nodes.
2. markdown-parser.spec.ts:
1. "Absolute link" - These tests cover more in-depth of link formats
that could possibly converted into links when parsed from a markdown
string. It verifies `autolink`, `validateLink`, and `normalizeLinkText`
in tokenizer rules and configurations set in the `markdown-parser.ts`.
3. markdown-serializer.spec.ts: 
1. These tests ensure only the text content(not `href`) in the `<a>` tag
is serialized to autolink markdown string.
2. It also covers how other marks (bold/italics) are ignored when the
link is wrapped within it.

<!---
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.
-->

## ✅ 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.

---------

Signed-off-by: Sai krishnan Perumal <[email protected]>
Signed-off-by: Sai krishnan Perumal <[email protected]>
Co-authored-by: Sai krishnan Perumal <[email protected]>
Co-authored-by: Sai krishnan Perumal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Backlog
Development

No branches or pull requests

4 participants