Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Blackbaud-ErikaMcVey committed Jan 31, 2024
1 parent 6cce2c6 commit 884e5be
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@
<input
formControlName="favoriteColor"
type="text"
[id]="inputId"
[presetColors]="swatches"
[skyColorpickerInput]="colorPicker"
/>
</sky-colorpicker>
</div>

<button class="sky-btn sky-btn-primary" type="submit">Submit</button>
<button class="sky-btn sky-btn-default" type="button" (click)="setId()">
Change input ID
</button>
</form>
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export class ColorpickerComponent {
'#DA9C9C',
];

public inputId = 'blah';

constructor(formBuilder: UntypedFormBuilder) {
this.reactiveForm = formBuilder.group({
favoriteColor: new UntypedFormControl('#f00'),
Expand All @@ -38,4 +40,8 @@ export class ColorpickerComponent {
const favoriteColor: string = controlValue.hex || controlValue;
alert('Your favorite color is: \n' + favoriteColor);
}

public setId(): void {
this.inputId = 'someId';
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
AfterContentChecked,
Directive,
ElementRef,
HostBinding,
Expand Down Expand Up @@ -52,7 +53,13 @@ const SKY_COLORPICKER_DEFAULT_COLOR = '#FFFFFF';
providers: [SKY_COLORPICKER_VALUE_ACCESSOR, SKY_COLORPICKER_VALIDATOR],
})
export class SkyColorpickerInputDirective
implements OnInit, OnChanges, ControlValueAccessor, Validator, OnDestroy
implements
OnInit,
OnChanges,
ControlValueAccessor,
Validator,
OnDestroy,
AfterContentChecked
{
/**
* Creates the colorpicker element and dropdown. Place this attribute on an `input` element
Expand Down Expand Up @@ -141,12 +148,13 @@ export class SkyColorpickerInputDirective
#svc: SkyColorpickerService;
#resourcesSvc: SkyLibResourcesService;
#injector: Injector;
#id: string | undefined;

#_disabled: boolean | undefined;
#_initialColor: string | undefined;

#colorpickerInputSvc = inject(SkyColorpickerInputService);
#idSvc = inject(SkyIdService);
readonly #colorpickerInputSvc = inject(SkyColorpickerInputService);
readonly #idSvc = inject(SkyIdService);

constructor(
elementRef: ElementRef,
Expand Down Expand Up @@ -179,14 +187,14 @@ export class SkyColorpickerInputDirective

public ngOnInit(): void {
const element = this.#elementRef.nativeElement;
let id = element.id;
this.#id = element.id;

if (!id) {
id = this.#idSvc.generateId();
this.#renderer.setAttribute(element, 'id', id);
if (!this.#id) {
this.#id = this.#idSvc.generateId();
this.#renderer.setAttribute(element, 'id', this.#id);
}

this.#colorpickerInputSvc.inputId.next(id);
this.#colorpickerInputSvc.inputId.next(this.#id);

this.#renderer.addClass(element, 'sky-form-control');
this.skyColorpickerInput.initialColor = this.initialColor;
Expand Down Expand Up @@ -230,6 +238,15 @@ export class SkyColorpickerInputDirective
}
}

public ngAfterContentChecked(): void {
const id = this.#elementRef.nativeElement?.id;

if (id && id !== this.#id) {
this.#colorpickerInputSvc.inputId.next(id);
this.#id = id;
}
}

public ngOnDestroy(): void {
if (this.#pickerChangedSubscription) {
this.#pickerChangedSubscription.unsubscribe();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,23 @@ describe('Colorpicker Component', () => {

expect(label).toBeVisible();
expect(label.textContent).toBe(labelText);
expect(label.getAttribute('for')).toBe(component.id);
});

it('should update the labelText for attribute if the input ID changes', () => {
const labelText = 'Label Text';
const newId = 'different-id';
component.labelText = labelText;

fixture.detectChanges();

const label = fixture.nativeElement.querySelector('.sky-control-label');
expect(label.getAttribute('for')).toBe(component.id);

component.id = newId;
fixture.detectChanges();

expect(label.getAttribute('for')).toBe(component.id);
});

it('should add icon overlay', fakeAsync(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,15 +273,7 @@ export class SkyColorpickerComponent implements OnInit, OnDestroy {
return this.#_colorpickerRef;
}

protected get inputId(): string | undefined {
return this.#_inputId;
}

protected set inputId(value: string | undefined) {
this.#_inputId = value;
this.#changeDetector.markForCheck();
}

protected inputId: string | undefined;
protected colorpickerId: string;
protected isOpen = false;
protected triggerButtonId: string;
Expand Down Expand Up @@ -319,12 +311,11 @@ export class SkyColorpickerComponent implements OnInit, OnDestroy {
#svc: SkyColorpickerService;
#themeSvc: SkyThemeService | undefined;

#colorpickerInputSvc = inject(SkyColorpickerInputService);
readonly #colorpickerInputSvc = inject(SkyColorpickerInputService);

#_backgroundColorForDisplay: string | undefined;
#_colorpickerRef: ElementRef | undefined;
#_disabled = false;
#_inputId: string | undefined;

constructor(
affixSvc: SkyAffixService,
Expand Down Expand Up @@ -390,6 +381,7 @@ export class SkyColorpickerComponent implements OnInit, OnDestroy {
.pipe(takeUntil(this.#ngUnsubscribe))
.subscribe((id) => {
this.inputId = id;
this.#changeDetector.markForCheck();
});

this.#addTriggerButtonEventListeners();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#colorPickerTest
>
<input
[id]="id"
[allowTransparency]="selectedTransparency"
[alphaChannel]="selectedHexType"
[disabled]="disabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export class ColorpickerTestComponent {
public inputType = 'text';
public selectedTransparency = true;
public disabled = false;
public id = 'id';

@ViewChild(SkyColorpickerComponent, {
static: true,
Expand Down

0 comments on commit 884e5be

Please sign in to comment.