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 6 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(): boolean {
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: BooleanValueOrAttribute) {
this.renderer.setProperty(this.elementRef.nativeElement, 'contenteditable', toBooleanProperty(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).toBeFalse();
expect(nativeElement.contenteditable).toBeFalse();
});
});

describe('with template string values', () => {
Expand All @@ -116,6 +121,7 @@ describe('Nimble anchor', () => {
type="${type1}"
appearance="prominent"
underline-hidden
contenteditable
>
</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).toBeTrue();
expect(nativeElement.contenteditable).toBeTrue();
});
});

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).toBeTrue();
expect(nativeElement.contenteditable).toBeTrue();

fixture.componentInstance.contentEditable = false;
fixture.detectChanges();

expect(directive.contentEditable).toBeFalse();
expect(nativeElement.contenteditable).toBeFalse();
});
});

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).toBeTrue();
expect(nativeElement.contenteditable).toBeTrue();

fixture.componentInstance.contentEditable = false;
fixture.detectChanges();

expect(directive.contentEditable).toBeFalse();
expect(nativeElement.contenteditable).toBeFalse();
});
});
});
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 bool? ContentEditable { get; set; }
m-akinc marked this conversation as resolved.
Show resolved Hide resolved

/// <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", 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
12 changes: 12 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,18 @@ export class Anchor extends AnchorBase {
*/
@attr
public appearance: AnchorAppearance;

/**
* @public
* @remarks
* HTML Attribute: 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({ mode: 'boolean' })
public contenteditable = false;
rajsite marked this conversation as resolved.
Show resolved Hide resolved
}

// 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
7 changes: 5 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,10 @@ import type { Anchor } from '.';
export const template: FoundationElementTemplate<
ViewTemplate<Anchor>,
AnchorOptions
> = (_context, definition) => html<Anchor>`<a
> = (_context, definition) => html<Anchor>`<div
m-akinc marked this conversation as resolved.
Show resolved Hide resolved
class="top-container"
?contenteditable="${x => x.contenteditable}"
><a
class="control"
part="control"
download="${x => x.download}"
Expand Down Expand Up @@ -74,4 +77,4 @@ AnchorOptions
@slotchange="${x => x.handleEndContentChange()}">
${definition.end || ''}
</slot
></span></a>`;
></span></a></div>`;
19 changes: 19 additions & 0 deletions packages/nimble-components/src/anchor/tests/anchor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,23 @@ describe('Anchor', () => {
});
}
});

describe('inner anchor isContentEditable', () => {
let innerAnchor: HTMLAnchorElement;

beforeEach(async () => {
await connect();
innerAnchor = element.shadowRoot!.querySelector('a')!;
});

it('is false by default', () => {
expect(innerAnchor.isContentEditable).toBeFalse();
});

it('is true when contenteditable set on host element', async () => {
element.setAttribute('contenteditable', '');
await waitForUpdatesAsync();
expect(innerAnchor.isContentEditable).toBeTrue();
});
});
});
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: boolean;
appearance: keyof typeof AnchorAppearance;
}

Expand All @@ -39,6 +40,7 @@ const metadata: Meta<AnchorArgs> = {
<span class="anchor-container">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 +58,17 @@ const metadata: Meta<AnchorArgs> = {
control: { type: 'radio' },
description:
'Set to `prominent` to make the anchor appear in a different color than normal text.'
},
contenteditable: {
description:
'Set this 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 can automatically adapt their behavior when within a `contenteditable` element, the `nimble-anchor` requires this attribute be explicitly set. When set, the anchor cannot be focused or operated.'
}
},
args: {
label: 'link',
href: 'https://nimble.ni.dev',
underlineHidden: false,
contenteditable: false,
appearance: 'default'
}
};
Expand Down
Loading