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

Nimble anchor inside "contenteditable" div is clickable #1502

Closed
Tracked by #1288
vivinkrishna-ni opened this issue Sep 8, 2023 · 0 comments · Fixed by #1684
Closed
Tracked by #1288

Nimble anchor inside "contenteditable" div is clickable #1502

vivinkrishna-ni opened this issue Sep 8, 2023 · 0 comments · Fixed by #1684
Labels
bug Something isn't working

Comments

@vivinkrishna-ni
Copy link
Contributor

🐛 Bug Report

When nimble-anchor is used inside a contenteditable div, the link is clickable, whereas the default behavior of the native HTML anchor tag is not clickable.

Note: Not sure if it is a bug or an expected behavior. Guide us in creating a correct issue for this behavior.

💻 Repro or Code Sample

A simple demo to reproduce: https://typescript-l63dbq.stackblitz.io

🤔 Expected Behavior

The default behavior of a nimble-anchor might be the same as that of a native HTML anchor inside a contenteditable div, meaning the link should not be clickable.

😯 Current Behavior

nimble-anchor is clickable and focusable inside contenteditable div.

💁 Possible Solution

🔦 Context

We are planning to render all links inside nimble-rich-text-editor as nimble-anchor as mentioned in this discussion: #1372 (comment). However, the rich text editor internally uses the contenteditable div to add rich text content to the editor. In this context, we should restrict the user from clicking any of the links that are being added to the editor.

🌍 Your Environment

  • OS & Device: Windows on PC
  • Browser: Google Chrome
  • Version: 116.0.5845.142
@vivinkrishna-ni vivinkrishna-ni added bug Something isn't working triage New issue that needs to be reviewed labels Sep 8, 2023
@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Sep 12, 2023
m-akinc added a commit that referenced this issue Dec 11, 2023
## 🤨 Rationale

Fixes #1502

## 👩‍💻 Implementation

By design, the content-editable state is [not
inherited](https://wicg.github.io/webcomponents/spec/shadow/#editing)
across the shadow boundary, but that is really what we want. To mimic
this, I introduced a new wrapper div* around the native anchor element
that we can set `contenteditable` on, so that the native anchor behaves
as we want. Ideally, we would set it based on the value of
`isContentEditable` of the host `nimble-anchor`. However, reacting to
changes in the host's `isContentEditable` state is a problem.
`isContentEditable` is a read-only property that is implemented by
walking the ancestry chain until an elmement with `contenteditable` set
is found. There is no practical way to detect changes to this value. We
will instead require that clients explicitly set `contenteditable` on
the `nimble-anchor` whenever it is inside a content-editable area. We
can reflect the value of that attribute to the wrapper div.

*Note that the new wrapper div is necessary, because if we instead set
`contenteditable` directly on the native anchor, it behaves differently
(than when it is contained by a content-editable element): it remains
focusable.

## 🧪 Testing

Manually tested in Storybook, and wrote new unit tests.

## ✅ Checklist

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

---------

Co-authored-by: Milan Raj <[email protected]>
@rajsite rajsite moved this from Backlog to Done in Nimble Design System Priorities Mar 7, 2024
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
Development

Successfully merging a pull request may close this issue.

2 participants