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

Make anchor behave like native anchor when contenteditable #1684

Merged
merged 16 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -116,6 +121,7 @@ describe('Nimble anchor', () => {
type="${type1}"
appearance="prominent"
underline-hidden
contenteditable="true"
>
</nimble-anchor>
`
Expand Down Expand Up @@ -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', () => {
Expand All @@ -199,6 +210,7 @@ describe('Nimble anchor', () => {
[type]="type"
[appearance]="appearance"
[underlineHidden]="underlineHidden"
[contentEditable]="contentEditable"
>
</nimble-anchor>
`
Expand All @@ -215,6 +227,7 @@ describe('Nimble anchor', () => {
public type = type1;
public appearance: AnchorAppearance = appearance1;
public underlineHidden = true;
public contentEditable = 'true';
}

let fixture: ComponentFixture<TestHostComponent>;
Expand Down Expand Up @@ -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', () => {
Expand All @@ -345,6 +369,7 @@ describe('Nimble anchor', () => {
[attr.type]="type"
[attr.appearance]="appearance"
[attr.underline-hidden]="underlineHidden"
[attr.contenteditable]="contentEditable"
>
</nimble-anchor>
`
Expand All @@ -361,6 +386,7 @@ describe('Nimble anchor', () => {
public type = type1;
public appearance: AnchorAppearance = appearance1;
public underlineHidden = true;
public contentEditable = 'true';
}

let fixture: ComponentFixture<TestHostComponent>;
Expand Down Expand Up @@ -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');
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Add contentEditable property to anchor directive",
"packageName": "@ni/nimble-angular",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Add ContentEditable property to anchor",
"packageName": "@ni/nimble-blazor",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Make anchor behave like native anchor when contenteditable",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
target="@Target"
type="@Type"
underline-hidden="@UnderlineHidden"
contenteditable="@ContentEditable"
appearance="@Appearance.ToAttributeValue()"
@attributes="AdditionalAttributes">
@ChildContent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
13 changes: 13 additions & 0 deletions packages/nimble-components/src/anchor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions packages/nimble-components/src/anchor/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export const styles = css`
font: ${linkFont};
}

.top-container {
display: contents;
}

[part='start'] {
display: none;
}
Expand Down
11 changes: 9 additions & 2 deletions packages/nimble-components/src/anchor/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -74,4 +81,4 @@ AnchorOptions
@slotchange="${x => x.handleEndContentChange()}">
${definition.end || ''}
</slot
></span></a>`;
></span></a></div>`;
73 changes: 72 additions & 1 deletion packages/nimble-components/src/anchor/tests/anchor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>`);
Expand Down Expand Up @@ -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();
});
});
});
12 changes: 11 additions & 1 deletion packages/nimble-components/src/anchor/tests/anchor.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ interface AnchorArgs {
label: string;
href: string;
underlineHidden: boolean;
contenteditable: string;
appearance: keyof typeof AnchorAppearance;
}

Expand All @@ -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>
`),
Expand All @@ -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. When set, the anchor cannot be focused or operated.'
m-akinc marked this conversation as resolved.
Show resolved Hide resolved
m-akinc marked this conversation as resolved.
Show resolved Hide resolved
}
},
args: {
label: 'link',
href: 'https://nimble.ni.dev',
underlineHidden: false,
contenteditable: 'false',
appearance: 'default'
}
};
Expand Down
Loading