Skip to content

Commit

Permalink
fix(label): allow signposts inside labels (#937)
Browse files Browse the repository at this point in the history
## PR Checklist

Please check if your PR fulfills the following requirements:

- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)
- [ ] If applicable, have a visual design approval

## PR Type

What kind of change does this PR introduce?

<!-- Please check the one that applies to this PR using "x". -->

- [X] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Documentation content changes
- [ ] Other... Please describe:

## What is the current behavior?

Currently if you put signposts inside labels they are not working due to
that `<label>` be default is spreading a click event to its children and
that's causing the signpost to automatically close and not appear.
**Example:**
If you have a label attached to an input or input inside a label and
click the label - the input will get focused.

Issue Number: CDE-9 and #271 

## What is the new behavior?

Signposts are now working inside `<label>`.

## Does this PR introduce a breaking change?

- [ ] Yes
- [X] No

<!-- If this PR contains a breaking change, please describe the impact
and migration path for existing applications below. -->

## Other information

---------

Co-authored-by: Kevin Buhmann <kbuhmann@vmware.com>
  • Loading branch information
dtsanevmw and kevinbuhmann authored Jan 7, 2025
1 parent 7812bc6 commit 92d623e
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 9 deletions.
2 changes: 1 addition & 1 deletion projects/angular/clarity.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2730,7 +2730,7 @@ export class ClrLabel implements OnInit, OnDestroy {
// (undocumented)
ngOnInit(): void;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<ClrLabel, "label", never, { "idInput": "id"; "forAttr": "for"; }, {}, never, never, false, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<ClrLabel, "label", never, { "idInput": "id"; "forAttr": "for"; }, {}, ["signpost"], never, false, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<ClrLabel, [{ optional: true; }, { optional: true; }, { optional: true; }, null, null]>;
}
Expand Down
51 changes: 51 additions & 0 deletions projects/angular/src/forms/common/label.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';

import { ClrIconModule } from '../../icon/icon.module';
import { ClrSignpostModule, ClrSignpostTrigger } from '../../popover';
import { ClrInputContainer } from '../input/input-container';
import { ClrLabel } from './label';
import { ControlIdService } from './providers/control-id.service';
Expand Down Expand Up @@ -47,6 +48,30 @@ class WrapperTest {}
})
class ExistingGridTest {}

@Component({
template: `
<label>
<clr-signpost>
<cds-icon id="signpost-trigger" shape="info-standard" clrSignpostTrigger>Trigger</cds-icon>
<clr-signpost-content *clrIfOpen>Signpost content</clr-signpost-content>
</clr-signpost>
</label>
`,
})
class SignpostTest {
@ViewChild(ClrSignpostTrigger) signpostTrigger: ClrSignpostTrigger;
}

@Component({
template: `
<label>
Test
<input type="text" />
</label>
`,
})
class DefaultClickBehaviorTest {}

export default function (): void {
describe('ClrLabel', () => {
it("doesn't crash if it is not used in an Angular form", function () {
Expand Down Expand Up @@ -175,5 +200,31 @@ export default function (): void {
const label = fixture.nativeElement.querySelector('label');
expect(label.getAttribute('for')).toBe('updatedFor');
});

it('signposts work inside labels', function () {
TestBed.configureTestingModule({
imports: [ClrSignpostModule, ClrIconModule],
declarations: [ClrLabel, SignpostTest],
});
const fixture = TestBed.createComponent(SignpostTest);
fixture.detectChanges();
const signpostTrigger = fixture.nativeElement.querySelector('#signpost-trigger');
signpostTrigger.click();
fixture.detectChanges();
expect(fixture.componentInstance.signpostTrigger.isOpen).toBe(true);
});

it('focus input on label click', function () {
TestBed.configureTestingModule({
declarations: [ClrLabel, DefaultClickBehaviorTest],
});
const fixture = TestBed.createComponent(DefaultClickBehaviorTest);
fixture.detectChanges();
const label = fixture.nativeElement.querySelector('label');
label.click();
fixture.detectChanges();
const input = fixture.nativeElement.querySelector('input');
expect(document.activeElement).toBe(input);
});
});
}
31 changes: 30 additions & 1 deletion projects/angular/src/forms/common/label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,21 @@
* The full license information can be found in LICENSE in the root directory of this project.
*/

import { Directive, ElementRef, HostBinding, Input, OnDestroy, OnInit, Optional, Renderer2 } from '@angular/core';
import {
ContentChild,
Directive,
ElementRef,
HostBinding,
HostListener,
Input,
OnDestroy,
OnInit,
Optional,
Renderer2,
} from '@angular/core';
import { Subscription } from 'rxjs';

import { ClrSignpost } from '../../popover';
import { ControlIdService } from './providers/control-id.service';
import { LayoutService } from './providers/layout.service';
import { NgControlService } from './providers/ng-control.service';
Expand All @@ -21,6 +33,7 @@ export class ClrLabel implements OnInit, OnDestroy {

@Input('for') @HostBinding('attr.for') forAttr: string;

@ContentChild(ClrSignpost, { read: ElementRef }) private signpost: ElementRef;
private enableGrid = true;
private subscriptions: Subscription[] = [];

Expand Down Expand Up @@ -69,4 +82,20 @@ export class ClrLabel implements OnInit, OnDestroy {
disableGrid() {
this.enableGrid = false;
}

/**
* Allowing signposts inside labels to work without disabling default behavior. <label> is spreading a click event to its children so signposts get
* automatically closed once clicked inside a <label>.
* @param event
*/
@HostListener('click', ['$event'])
private onClick(event) {
this.preventDefaultOnSignpostTarget(event);
}

private preventDefaultOnSignpostTarget(event) {
if (this.signpost && this.signpost.nativeElement && this.signpost.nativeElement.contains(event.target)) {
event.preventDefault();
}
}
}
56 changes: 49 additions & 7 deletions projects/demo/src/app/forms/layout-angular/layout-angular.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ <h1>Angular - {{layout}} - {{ labelSize ? 'grid' : 'no grid' }}</h1>

<form clrForm [clrLayout]="layout" [clrLabelSize]="labelSize">
<clr-checkbox-container>
<label>Checkbox</label>
<label>
Checkbox
<clr-signpost>
<cds-icon shape="info-standard" clrSignpostTrigger>Trigger</cds-icon>
<clr-signpost-content *clrIfOpen>Signpost content</clr-signpost-content>
</clr-signpost>
</label>
<clr-checkbox-wrapper>
<label>Option 1 2 3</label>
<input type="checkbox" clrCheckbox />
Expand All @@ -24,19 +30,43 @@ <h1>Angular - {{layout}} - {{ labelSize ? 'grid' : 'no grid' }}</h1>
</clr-checkbox-wrapper>
</clr-checkbox-container>
<clr-date-container>
<label>Date picker</label>
<label>
Date picker
<clr-signpost>
<cds-icon shape="info-standard" clrSignpostTrigger>Trigger</cds-icon>
<clr-signpost-content *clrIfOpen>Signpost content</clr-signpost-content>
</clr-signpost>
</label>
<input clrDate />
</clr-date-container>
<clr-input-container>
<label>Input</label>
<label>
Input
<clr-signpost>
<cds-icon shape="info-standard" clrSignpostTrigger>Trigger</cds-icon>
<clr-signpost-content *clrIfOpen>Signpost content</clr-signpost-content>
</clr-signpost>
</label>
<input clrInput />
</clr-input-container>
<clr-password-container>
<label>Password</label>
<label>
Password
<clr-signpost>
<cds-icon shape="info-standard" clrSignpostTrigger>Trigger</cds-icon>
<clr-signpost-content *clrIfOpen>Signpost content</clr-signpost-content>
</clr-signpost>
</label>
<input type="password" autocomplete="current-password" clrPassword />
</clr-password-container>
<clr-radio-container>
<label>Radio</label>
<label>
Radio
<clr-signpost>
<cds-icon shape="info-standard" clrSignpostTrigger>Trigger</cds-icon>
<clr-signpost-content *clrIfOpen>Signpost content</clr-signpost-content>
</clr-signpost>
</label>
<clr-radio-wrapper>
<label>Option 1</label>
<input type="radio" clrRadio />
Expand All @@ -51,15 +81,27 @@ <h1>Angular - {{layout}} - {{ labelSize ? 'grid' : 'no grid' }}</h1>
</clr-radio-wrapper>
</clr-radio-container>
<clr-select-container>
<label>Select</label>
<label>
Select
<clr-signpost>
<cds-icon shape="info-standard" clrSignpostTrigger>Trigger</cds-icon>
<clr-signpost-content *clrIfOpen>Signpost content</clr-signpost-content>
</clr-signpost>
</label>
<select clrSelect>
<option value="1">Option 1</option>
<option value="2">Option 2</option>
<option value="3">Option 3</option>
</select>
</clr-select-container>
<clr-textarea-container>
<label>Textarea</label>
<label>
Textarea
<clr-signpost>
<cds-icon shape="info-standard" clrSignpostTrigger>Trigger</cds-icon>
<clr-signpost-content *clrIfOpen>Signpost content</clr-signpost-content>
</clr-signpost>
</label>
<textarea clrTextarea></textarea>
</clr-textarea-container>
<clr-toggle-container>
Expand Down

0 comments on commit 92d623e

Please sign in to comment.