Skip to content

fix(*): icon service doesn't work with scoped themes #15150

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Dec 10, 2024
Merged

fix(*): icon service doesn't work with scoped themes #15150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a5d56d1
fix(*): icon service doesn't work with scoped themes
simeonoff Dec 2, 2024
759b916
spec(combo): fix crashing tests
simeonoff Dec 2, 2024
c6a47ea
refactor(theme-token): abstract away token type as class
simeonoff Dec 3, 2024
3213f4d
spec(icon-service): add integration tests for runtime refence switching
simeonoff Dec 3, 2024
a11f6fc
refactor(theme-token): simplify implementation
simeonoff Dec 4, 2024
7fb0144
chore(input-group): remove console.log statement
simeonoff Dec 4, 2024
6184e67
Merge branch '18.2.x' into simeonoff/18.2.x-fix-15094
simeonoff Dec 6, 2024
6f1751e
chore(demos): make icon samples standalone
simeonoff Dec 6, 2024
01de315
spec(icon-service): add standalone to new test component
simeonoff Dec 6, 2024
4d9d2b4
refactor(*): address PR review comments
simeonoff Dec 9, 2024
4f8bd23
refactor(theme-token): onChange returns subscription
simeonoff Dec 9, 2024
c7e5017
test(combo): cleanup redundant icon service in unit tests
damyanpetev Dec 9, 2024
08d2fa0
Merge branch '18.2.x' into simeonoff/18.2.x-fix-15094
kdinev Dec 9, 2024
be24bce
refactor(checkbox, input): don't set the theme token to the local theme
simeonoff Dec 10, 2024
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
44 changes: 26 additions & 18 deletions projects/igniteui-angular/src/lib/checkbox/checkbox.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@ import {
Self,
booleanAttribute,
inject,
DestroyRef
DestroyRef,
Inject
} from '@angular/core';
import { ControlValueAccessor, NgControl, Validators } from '@angular/forms';
import { IgxRippleDirective } from '../directives/ripple/ripple.directive';
import { IBaseEventArgs, mkenum } from '../core/utils';
import { IBaseEventArgs, getComponentTheme, mkenum } from '../core/utils';
import { EditorProvider, EDITOR_PROVIDER } from '../core/edit-provider';
import { noop, Subject, Subscription } from 'rxjs';
import { noop, Subject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';
import { IgxTheme, ThemeService } from '../services/theme/theme.service';
import { IgxTheme, THEME_TOKEN, ThemeToken } from '../services/theme/theme.token';

export const LabelPosition = /*@__PURE__*/mkenum({
BEFORE: 'before',
Expand Down Expand Up @@ -493,28 +494,40 @@ export class IgxCheckboxComponent implements EditorProvider, AfterViewInit, Cont
*/
private _required = false;
private elRef = inject(ElementRef);
private _theme$ = new Subject<IgxTheme>();
private _subscription: Subscription;
private destroyRef = inject(DestroyRef);

constructor(
protected cdr: ChangeDetectorRef,
protected renderer: Renderer2,
protected themeService: ThemeService,
@Inject(THEME_TOKEN)
protected themeToken: ThemeToken,
@Optional() @Self() public ngControl: NgControl,
) {
if (this.ngControl !== null) {
this.ngControl.valueAccessor = this;
}

this.theme = this.themeService.globalTheme;
this.theme = this.themeToken.theme;

this._subscription = this._theme$.asObservable().subscribe(value => {
this.theme = value as IgxTheme;
this.cdr.detectChanges();
const { unsubscribe } = this.themeToken.onChange((theme) => {
if (this.theme !== theme) {
this.theme = theme;
this.cdr.detectChanges();
}
});

this.destroyRef.onDestroy(() => this._subscription.unsubscribe());
this.destroyRef.onDestroy(() => unsubscribe);
}

private setComponentTheme() {
if(!this.themeToken.preferToken) {
const theme = getComponentTheme(this.elRef.nativeElement);

if (theme && theme !== this.theme) {
this.theme = theme;
this.cdr.markForCheck();
}
}
}

/**
Expand All @@ -531,12 +544,7 @@ export class IgxCheckboxComponent implements EditorProvider, AfterViewInit, Cont
}
}

const theme = this.themeService.getComponentTheme(this.elRef);

if (theme) {
this._theme$.next(theme);
this.cdr.markForCheck();
}
this.setComponentTheme();
}

/** @hidden @internal */
Expand Down
93 changes: 23 additions & 70 deletions projects/igniteui-angular/src/lib/combo/combo.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import { SortingDirection } from '../data-operations/sorting-strategy';
import { IForOfState } from '../directives/for-of/for_of.directive';
import { IgxInputState } from '../directives/input/input.directive';
import { IgxIconService } from '../icon/public_api';
import { IgxLabelDirective } from '../input-group/public_api';
import { AbsoluteScrollStrategy, ConnectedPositioningStrategy } from '../services/public_api';
import { configureTestSuite } from '../test-utils/configure-suite';
Expand Down Expand Up @@ -87,7 +86,6 @@
get: mockNgControl
});
mockSelection.get.and.returnValue(new Set([]));
const mockIconService = new IgxIconService(null, null, null, null);
const mockDocument = jasmine.createSpyObj('DOCUMENT', [], { 'defaultView': { getComputedStyle: () => null }});

it('should correctly implement interface methods - ControlValueAccessor ', () => {
Expand All @@ -98,11 +96,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);

spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
expect(mockInjector.get).toHaveBeenCalledWith(NgControl, null);
combo.registerOnChange(mockNgControl.registerOnChangeCb);
Expand Down Expand Up @@ -146,12 +142,10 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);

const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['open', 'close', 'toggle']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.dropdown = dropdown;
dropdown.collapsed = true;
Expand Down Expand Up @@ -179,12 +173,10 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdownContainer = { nativeElement: { focus: () => { } } };
combo['dropdownContainer'] = dropdownContainer;
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
spyOn(combo, 'focusSearchInput');

combo.autoFocusSearch = false;
Expand All @@ -207,11 +199,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['toggle']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.dropdown = dropdown;
const defaultSettings = (combo as any)._overlaySettings;
Expand All @@ -234,10 +224,8 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.valueKey = 'field';
expect(combo.displayKey).toEqual(combo.valueKey);
Expand All @@ -253,10 +241,8 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
mockSelection.select_items.calls.reset();
Expand All @@ -281,11 +267,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = complexData;
combo.valueKey = 'country';
Expand Down Expand Up @@ -336,11 +320,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
combo.dropdown = dropdown;
Expand Down Expand Up @@ -375,10 +357,8 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
spyOn(combo.opening, 'emit').and.callThrough();
spyOn(combo.closing, 'emit').and.callThrough();
Expand Down Expand Up @@ -423,11 +403,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
combo.dropdown = dropdown;
Expand Down Expand Up @@ -524,11 +502,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = complexData;
combo.valueKey = 'country';
Expand Down Expand Up @@ -570,11 +546,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = complexData;
combo.valueKey = 'country';
Expand Down Expand Up @@ -637,11 +611,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
combo.dropdown = dropdown;
Expand All @@ -666,11 +638,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
combo.dropdown = dropdown;
Expand Down Expand Up @@ -720,11 +690,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
combo.dropdown = dropdown;
Expand All @@ -745,10 +713,8 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
let errorMessage = '';
try {
Expand All @@ -769,10 +735,8 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
let errorMessage = '';
try {
Expand All @@ -793,11 +757,9 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
combo.dropdown = dropdown;
Expand Down Expand Up @@ -841,10 +803,8 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
combo.disableFiltering = false;
Expand All @@ -866,12 +826,10 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['open', 'close', 'toggle']);
const spyObj = jasmine.createSpyObj('event', ['stopPropagation', 'preventDefault']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.dropdown = dropdown;
dropdown.collapsed = true;
Expand All @@ -889,12 +847,10 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
const spyObj = jasmine.createSpyObj('event', ['stopPropagation']);
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
combo.ngOnInit();
combo.data = data;
combo.dropdown = dropdown;
Expand All @@ -916,16 +872,14 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
const dropdown = jasmine.createSpyObj('IgxComboDropDownComponent', ['selectItem']);
const mockVirtDir = jasmine.createSpyObj('virtDir', ['scrollTo']);
const mockInput = jasmine.createSpyObj('mockInput', [], {
nativeElement: jasmine.createSpyObj('mockElement', ['focus'])
});
spyOn(combo.addition, 'emit').and.callThrough();
spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null);
const subParams: { cancel: boolean; newValue: string; modify: boolean } = {
cancel: false,
modify: false,
Expand Down Expand Up @@ -1023,8 +977,7 @@
mockComboService,
mockDocument,
null,
mockInjector,
mockIconService
mockInjector
);
combo.ngOnDestroy();
expect(mockComboService.clear).toHaveBeenCalled();
Expand Down Expand Up @@ -1758,8 +1711,8 @@
combo.virtualScrollContainer.scrollTo(51);
await firstValueFrom(combo.virtualScrollContainer.chunkLoad);
fixture.detectChanges();
let items = fixture.debugElement.queryAll(By.css(`.${CSS_CLASS_DROPDOWNLISTITEM}`));

Check warning on line 1714 in projects/igniteui-angular/src/lib/combo/combo.component.spec.ts

View workflow job for this annotation

GitHub Actions / run-tests (18.x) 

'items' is never reassigned. Use 'const' instead

Check warning on line 1714 in projects/igniteui-angular/src/lib/combo/combo.component.spec.ts

View workflow job for this annotation

GitHub Actions / run-tests (20.x) 

'items' is never reassigned. Use 'const' instead
let lastItem = items[items.length - 1].componentInstance;

Check warning on line 1715 in projects/igniteui-angular/src/lib/combo/combo.component.spec.ts

View workflow job for this annotation

GitHub Actions / run-tests (18.x) 

'lastItem' is never reassigned. Use 'const' instead

Check warning on line 1715 in projects/igniteui-angular/src/lib/combo/combo.component.spec.ts

View workflow job for this annotation

GitHub Actions / run-tests (20.x) 

'lastItem' is never reassigned. Use 'const' instead
expect(lastItem).toBeDefined();
lastItem.clicked(mockClick);
fixture.detectChanges();
Expand Down
Loading
Loading