Make anchor behave like native anchor when contenteditable (#1684)

## 🤨 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]>
ni · Dec 11, 2023 · 487c344 · 487c344
1 parent d072c81
commit 487c344
Show file tree

Hide file tree

Showing 13 changed files with 200 additions and 4 deletions.
diff --git a/...lar-workspace/projects/ni/nimble-angular/src/directives/anchor/nimble-anchor.directive.ts b/...lar-workspace/projects/ni/nimble-angular/src/directives/anchor/nimble-anchor.directive.ts
@@ -33,6 +33,16 @@ export class NimbleAnchorDirective extends NimbleAnchorBaseDirective<Anchor> {
         this.renderer.setProperty(this.elementRef.nativeElement, 'underlineHidden', toBooleanProperty(value));
     }
 
+    public get contentEditable(): string {
+        return this.elementRef.nativeElement.contentEditable;
+    }
+
+    // Renaming because property should have camel casing, but attribute should not
+    // eslint-disable-next-line @angular-eslint/no-input-rename
+    @Input('contenteditable') public set contentEditable(value: string) {
+        this.renderer.setProperty(this.elementRef.nativeElement, 'contentEditable', value);
+    }
+
     public constructor(renderer: Renderer2, elementRef: ElementRef<Anchor>) {
         super(renderer, elementRef);
     }

diff --git a/...ce/projects/ni/nimble-angular/src/directives/anchor/tests/nimble-anchor.directive.spec.ts b/...ce/projects/ni/nimble-angular/src/directives/anchor/tests/nimble-anchor.directive.spec.ts
@@ -101,6 +101,11 @@ describe('Nimble anchor', () => {
             expect(directive.underlineHidden).toBeFalse();
             expect(nativeElement.underlineHidden).toBeFalse();
         });
+
+        it('has expected defaults for contentEditable', () => {
+            expect(directive.contentEditable).toBeUndefined();
+            expect(nativeElement.contentEditable).toBeUndefined();
+        });
     });
 
     describe('with template string values', () => {
@@ -116,6 +121,7 @@ describe('Nimble anchor', () => {
                     type="${type1}"
                     appearance="prominent"
                     underline-hidden
+                    contenteditable="true"
                     >
                 </nimble-anchor>
             `
@@ -184,6 +190,11 @@ describe('Nimble anchor', () => {
             expect(directive.underlineHidden).toBeTrue();
             expect(nativeElement.underlineHidden).toBeTrue();
         });
+
+        it('will use template string values for contentEditable', () => {
+            expect(directive.contentEditable).toEqual('true');
+            expect(nativeElement.contentEditable).toEqual('true');
+        });
     });
 
     describe('with property bound values', () => {
@@ -199,6 +210,7 @@ describe('Nimble anchor', () => {
                     [type]="type"
                     [appearance]="appearance"
                     [underlineHidden]="underlineHidden"
+                    [contentEditable]="contentEditable"
                     >
                 </nimble-anchor>
             `
@@ -215,6 +227,7 @@ describe('Nimble anchor', () => {
             public type = type1;
             public appearance: AnchorAppearance = appearance1;
             public underlineHidden = true;
+            public contentEditable = 'true';
         }
 
         let fixture: ComponentFixture<TestHostComponent>;
@@ -330,6 +343,17 @@ describe('Nimble anchor', () => {
             expect(directive.underlineHidden).toBeFalse();
             expect(nativeElement.underlineHidden).toBeFalse();
         });
+
+        it('can be configured with property binding for contentEditable', () => {
+            expect(directive.contentEditable).toEqual('true');
+            expect(nativeElement.contentEditable).toEqual('true');
+
+            fixture.componentInstance.contentEditable = 'false';
+            fixture.detectChanges();
+
+            expect(directive.contentEditable).toEqual('false');
+            expect(nativeElement.contentEditable).toEqual('false');
+        });
     });
 
     describe('with attribute bound values', () => {
@@ -345,6 +369,7 @@ describe('Nimble anchor', () => {
                     [attr.type]="type"
                     [attr.appearance]="appearance"
                     [attr.underline-hidden]="underlineHidden"
+                    [attr.contenteditable]="contentEditable"
                     >
                 </nimble-anchor>
             `
@@ -361,6 +386,7 @@ describe('Nimble anchor', () => {
             public type = type1;
             public appearance: AnchorAppearance = appearance1;
             public underlineHidden = true;
+            public contentEditable = 'true';
         }
 
         let fixture: ComponentFixture<TestHostComponent>;
@@ -476,5 +502,16 @@ describe('Nimble anchor', () => {
             expect(directive.underlineHidden).toBeFalse();
             expect(nativeElement.underlineHidden).toBeFalse();
         });
+
+        it('can be configured with attribute binding for contentEditable', () => {
+            expect(directive.contentEditable).toEqual('true');
+            expect(nativeElement.contentEditable).toEqual('true');
+
+            fixture.componentInstance.contentEditable = 'false';
+            fixture.detectChanges();
+
+            expect(directive.contentEditable).toEqual('false');
+            expect(nativeElement.contentEditable).toEqual('false');
+        });
     });
 });
diff --git a/change/@ni-nimble-angular-af4cc1a5-1c53-4eac-8cba-b38dcd8fe376.json b/change/@ni-nimble-angular-af4cc1a5-1c53-4eac-8cba-b38dcd8fe376.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Add contentEditable property to anchor directive",
+  "packageName": "@ni/nimble-angular",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@ni-nimble-blazor-21d307d5-1145-4e6c-8be4-dd65b8ca424c.json b/change/@ni-nimble-blazor-21d307d5-1145-4e6c-8be4-dd65b8ca424c.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Add ContentEditable property to anchor",
+  "packageName": "@ni/nimble-blazor",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@ni-nimble-components-be72d648-96c1-403d-80e8-2a68c3752207.json b/change/@ni-nimble-components-be72d648-96c1-403d-80e8-2a68c3752207.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Make anchor behave like native anchor when contenteditable",
+  "packageName": "@ni/nimble-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleAnchor.razor b/packages/nimble-blazor/NimbleBlazor/Components/NimbleAnchor.razor
@@ -9,6 +9,7 @@
     target="@Target"
     type="@Type"
     underline-hidden="@UnderlineHidden"
+    contenteditable="@ContentEditable"
     appearance="@Appearance.ToAttributeValue()"
     @attributes="AdditionalAttributes">
     @ChildContent

diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleAnchor.razor.cs b/packages/nimble-blazor/NimbleBlazor/Components/NimbleAnchor.razor.cs
@@ -10,6 +10,12 @@ public partial class NimbleAnchor : NimbleAnchorBase
     [Parameter]
     public bool? UnderlineHidden { get; set; }
 
+    /// <summary>
+    /// Whether the anchor should behave like it is in an editable region.
+    /// </summary>
+    [Parameter]
+    public string? ContentEditable { get; set; }
+
     /// <summary>
     /// The appearance of the anchor.
     /// </summary>

diff --git a/packages/nimble-blazor/Tests/NimbleBlazor.Tests/Unit/Components/NimbleAnchorTests.cs b/packages/nimble-blazor/Tests/NimbleBlazor.Tests/Unit/Components/NimbleAnchorTests.cs
@@ -41,6 +41,22 @@ public void AnchorAppearance_AttributeIsSet(AnchorAppearance value, string expec
         Assert.Contains(expectedMarkup, anchor.Markup);
     }
 
+    [Fact]
+    public void AnchorContentEditable_AttributeIsSet()
+    {
+        var anchor = RenderWithPropertySet(x => x.ContentEditable, "true");
+
+        Assert.Contains("contenteditable=\"true\"", anchor.Markup);
+    }
+
+    [Fact]
+    public void AnchorUnderlineHidden_AttributeIsSet()
+    {
+        var anchor = RenderWithPropertySet(x => x.UnderlineHidden, true);
+
+        Assert.Contains("underline-hidden", anchor.Markup);
+    }
+
     private IRenderedComponent<NimbleAnchor> RenderWithPropertySet<TProperty>(Expression<Func<NimbleAnchor, TProperty>> propertyGetter, TProperty propertyValue)
     {
         var context = new TestContext();

diff --git a/packages/nimble-components/src/anchor/index.ts b/packages/nimble-components/src/anchor/index.ts
@@ -34,6 +34,19 @@ export class Anchor extends AnchorBase {
      */
     @attr
     public appearance: AnchorAppearance;
+
+    /**
+     * @public
+     * @remarks
+     * HTML Attribute: contenteditable
+     * See https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/contenteditable
+     *
+     * Ideally, proper support for contenteditable should come from FAST.
+     * I have filed bug https://github.com/microsoft/fast/issues/6870 to them.
+     * If/when it is fixed, we can remove this workaround.
+     */
+    @attr({ attribute: 'contenteditable' })
+    public override contentEditable!: string;
 }
 
 // FoundationAnchor already applies the StartEnd mixin, so we don't need to do it here.

diff --git a/packages/nimble-components/src/anchor/styles.ts b/packages/nimble-components/src/anchor/styles.ts
@@ -20,6 +20,10 @@ export const styles = css`
         font: ${linkFont};
     }
 
+    .top-container {
+        display: contents;
+    }
+
     [part='start'] {
         display: none;
     }

diff --git a/packages/nimble-components/src/anchor/template.ts b/packages/nimble-components/src/anchor/template.ts
@@ -9,7 +9,14 @@ import type { Anchor } from '.';
 export const template: FoundationElementTemplate<
 ViewTemplate<Anchor>,
 AnchorOptions
-> = (_context, definition) => html<Anchor>`<a
+> = (_context, definition) => html<Anchor>`${
+    /* top-container div is necessary because setting contenteditable directly on the native anchor instead
+       leaves it focusable, unlike the behavior you get when the anchor is _within_ a contenteditable element.
+    */ ''
+}<div
+        class="top-container"
+        contenteditable="${x => x.contentEditable}"
+    ><a
         class="control"
         part="control"
         download="${x => x.download}"
@@ -74,4 +81,4 @@ AnchorOptions
             @slotchange="${x => x.handleEndContentChange()}">
             ${definition.end || ''}
         </slot
-    ></span></a>`;
+    ></span></a></div>`;
diff --git a/packages/nimble-components/src/anchor/tests/anchor.spec.ts b/packages/nimble-components/src/anchor/tests/anchor.spec.ts
@@ -2,7 +2,10 @@ import { html } from '@microsoft/fast-element';
 import { Anchor, anchorTag } from '..';
 import { waitForUpdatesAsync } from '../../testing/async-helpers';
 import { fixture, Fixture } from '../../utilities/tests/fixture';
-import { getSpecTypeByNamedList } from '../../utilities/tests/parameterized';
+import {
+    getSpecTypeByNamedList,
+    parameterizeNamedList
+} from '../../utilities/tests/parameterized';
 
 async function setup(): Promise<Fixture<Anchor>> {
     return fixture<Anchor>(html`<nimble-anchor></nimble-anchor>`);
@@ -91,4 +94,72 @@ describe('Anchor', () => {
             });
         }
     });
+
+    describe('contenteditable behavior', () => {
+        let innerAnchor: HTMLAnchorElement;
+
+        beforeEach(async () => {
+            await connect();
+            innerAnchor = element.shadowRoot!.querySelector('a')!;
+        });
+
+        it('has undefined property value and inner anchor isContentEditable is false by default', () => {
+            expect(element.contentEditable).toBeUndefined();
+            expect(innerAnchor.isContentEditable).toBeFalse();
+        });
+
+        const interestingValues = [
+            { name: '', expected: true, skipTag: '' },
+            { name: 'true', expected: true, skipTag: '' },
+            { name: 'false', expected: false, skipTag: '' },
+            { name: 'plaintext-only', expected: true, skipTag: '#SkipFirefox' },
+            { name: 'inherit', expected: false, skipTag: '' },
+            { name: 'badvalue', expected: false, skipTag: '' }
+        ] as const;
+
+        parameterizeNamedList(interestingValues, (spec, name, value) => {
+            spec(
+                `inner anchor isContentEditable is ${value.expected.toString()} when attribute set to "${name}" ${
+                    value.skipTag
+                }`,
+                async () => {
+                    element.setAttribute('contenteditable', name);
+                    await waitForUpdatesAsync();
+                    expect(innerAnchor.isContentEditable).toEqual(
+                        value.expected
+                    );
+                }
+            );
+        });
+
+        parameterizeNamedList(interestingValues, (spec, name, value) => {
+            spec(
+                `inner anchor isContentEditable is ${value.expected.toString()} when property set to "${name}" ${
+                    value.skipTag
+                }`,
+                async () => {
+                    element.contentEditable = name;
+                    await waitForUpdatesAsync();
+                    expect(innerAnchor.isContentEditable).toEqual(
+                        value.expected
+                    );
+                }
+            );
+        });
+    });
+
+    describe('with contenteditable without value', () => {
+        async function setupWithContenteditable(): Promise<Fixture<Anchor>> {
+            return fixture<Anchor>(
+                html`<nimble-anchor contenteditable></nimble-anchor>`
+            );
+        }
+
+        it('acts like value is "true"', async () => {
+            ({ element, connect, disconnect } = await setupWithContenteditable());
+            await connect();
+            const innerAnchor = element.shadowRoot!.querySelector('a')!;
+            expect(innerAnchor.isContentEditable).toBeTrue();
+        });
+    });
 });
diff --git a/packages/nimble-components/src/anchor/tests/anchor.stories.ts b/packages/nimble-components/src/anchor/tests/anchor.stories.ts
@@ -15,6 +15,7 @@ interface AnchorArgs {
     label: string;
     href: string;
     underlineHidden: boolean;
+    contenteditable: string;
     appearance: keyof typeof AnchorAppearance;
 }
 
@@ -34,11 +35,13 @@ const metadata: Meta<AnchorArgs> = {
         <style class='code-hide'>
             .anchor-container {
                 font: var(${bodyFont.cssCustomProperty});
+                outline: none;
             }
         </style>
-        <span class="anchor-container">Click on the <${anchorTag}
+        <span class="anchor-container" contenteditable=${x => x.contenteditable}>Click on the <${anchorTag}
             href=${x => (x.href !== '' ? x.href : null)}
             ?underline-hidden=${x => x.underlineHidden}
+            contenteditable=${x => x.contenteditable}
             appearance=${x => x.appearance}
         >${x => x.label}</${anchorTag}> to navigate.</span>
     `),
@@ -56,12 +59,19 @@ const metadata: Meta<AnchorArgs> = {
             control: { type: 'radio' },
             description:
                 'Set to `prominent` to make the anchor appear in a different color than normal text.'
+        },
+        contenteditable: {
+            options: ['false', 'true'],
+            control: { type: 'radio' },
+            description:
+                'Set this to the string "true" (or set the attribute without any value)  when the anchor is within an editable region (i.e. element/hierarchy with [contenteditable](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/contenteditable)). Whereas native elements inherit their `contenteditable` value by default, the `nimble-anchor` requires this attribute be explicitly set.'
         }
     },
     args: {
         label: 'link',
         href: 'https://nimble.ni.dev',
         underlineHidden: false,
+        contenteditable: 'false',
         appearance: 'default'
     }
 };