Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

refactor: Adapt foundations for class-object adapters #6256

Closed
wants to merge 16 commits into from
Closed

refactor: Adapt foundations for class-object adapters #6256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9ab51f5
refactor(material-experimental-foundations): Adapt foundations for cl…
ngwattcos Jul 23, 2020
074a61c
refactor(material-experimental): change foundation-base to store adap…
ngwattcos Jul 24, 2020
b226666
refactor(components): adapt all remaining foundations to use class ob…
ngwattcos Jul 24, 2020
ba40061
refactor(mdc-components): simplify overriding constructor
ngwattcos Jul 27, 2020
30abe58
fix(chips/chip): resolve build errors
ngwattcos Jul 27, 2020
c0ad50f
fix(tests): remove verifyDefaultAdapter tests for default adapters
ngwattcos Aug 6, 2020
89ce243
fix(tests): remove remaining default adapter tests
ngwattcos Aug 6, 2020
535d97d
Merge branch 'master' into foundation-use-class-adapter
ngwattcos Aug 8, 2020
5395e39
fix(segmented-button): fix build errors
ngwattcos Aug 8, 2020
1000147
fix(segmented-button): fix some test failures
ngwattcos Aug 10, 2020
b692a91
fix(components): add back default adapters
ngwattcos Aug 20, 2020
2c7ebf2
lint(mdc-dom/announcer): fix lint issues that also solves unit test f…
ngwattcos Aug 20, 2020
a334b06
Merge branch 'master' into foundation-use-class-adapter
ngwattcos Aug 20, 2020
5758257
lint(foundations): remove trailing whitespace
ngwattcos Aug 20, 2020
ca52634
lint(mdc-chip): remove trailing whitespace
ngwattcos Aug 20, 2020
2d79b01
fix(mdc-chips/chip-set): change property access to use string literals
ngwattcos Aug 20, 2020
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
2 changes: 1 addition & 1 deletion packages/mdc-base/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class MDCFoundation<AdapterType extends {} = {}> {
return {};
}

constructor(protected adapter: AdapterType = {} as AdapterType) {}
constructor(protected readonly adapter: AdapterType = {} as AdapterType) {}

init() {
// Subclasses should override this method to perform initialization routines (registering events, etc.)
Expand Down
8 changes: 2 additions & 6 deletions packages/mdc-checkbox/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export class MDCCheckboxFoundation extends MDCFoundation<MDCCheckboxAdapter> {
return numbers;
}

static get defaultAdapter(): MDCCheckboxAdapter {
return {
static get defaultAdapter(): MDCCheckboxAdapter {
return {
addClass: () => undefined,
forceLayout: () => undefined,
hasNativeControl: () => false,
Expand All @@ -58,10 +58,6 @@ export class MDCCheckboxFoundation extends MDCFoundation<MDCCheckboxAdapter> {
private animEndLatchTimer_ = 0;
private enableAnimationEndHandler_ = false;

constructor(adapter?: Partial<MDCCheckboxAdapter>) {
super({...MDCCheckboxFoundation.defaultAdapter, ...adapter});
}

init() {
this.currentCheckState_ = this.determineCheckState_();
this.updateAriaChecked_();
Expand Down
16 changes: 0 additions & 16 deletions packages/mdc-checkbox/test/foundation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import 'jasmine';

import {verifyDefaultAdapter} from '../../../testing/helpers/foundation';
import {setUpFoundationTest, setUpMdcTestEnvironment} from '../../../testing/helpers/setup';
import {cssClasses, numbers, strings} from '../constants';
import MDCCheckboxFoundation from '../foundation';
Expand Down Expand Up @@ -100,21 +99,6 @@ describe('MDCCheckboxFoundation', () => {
expect(strings).toEqual(MDCCheckboxFoundation.strings);
});

it('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCCheckboxFoundation, [
'addClass',
'removeClass',
'setNativeControlAttr',
'removeNativeControlAttr',
'forceLayout',
'isAttachedToDOM',
'isIndeterminate',
'isChecked',
'hasNativeControl',
'setNativeControlDisabled',
]);
});

it('#init adds the upgraded class to the root element', () => {
const {foundation, mockAdapter} = setupTest();
foundation.init();
Expand Down
4 changes: 0 additions & 4 deletions packages/mdc-chips/chip-set/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
*/
private selectedChipIds_: string[] = [];

constructor(adapter?: Partial<MDCChipSetAdapter>) {
super({...MDCChipSetFoundation.defaultAdapter, ...adapter});
}

/**
* Returns an array of the IDs of all selected chips.
*/
Expand Down
20 changes: 2 additions & 18 deletions packages/mdc-chips/chip-set/test/foundation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@
*/


import {EventSource, strings} from '../../../mdc-chips/chip/constants';
import {MDCChipSetFoundation} from '../../../mdc-chips/chip-set/foundation';
import {verifyDefaultAdapter} from '../../../../testing/helpers/foundation';
import {setUpFoundationTest} from '../../../../testing/helpers/setup';
import {MDCChipSetFoundation} from '../../../mdc-chips/chip-set/foundation';
import {EventSource, strings} from '../../../mdc-chips/chip/constants';

const {cssClasses} = MDCChipSetFoundation;

Expand All @@ -38,21 +37,6 @@ describe('MDCChipSetFoundation', () => {
expect('cssClasses' in MDCChipSetFoundation).toBeTruthy();
});

it('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCChipSetFoundation, [
'hasClass',
'removeChipAtIndex',
'selectChipAtIndex',
'focusChipPrimaryActionAtIndex',
'focusChipTrailingActionAtIndex',
'getIndexOfChipById',
'isRTL',
'getChipListCount',
'removeFocusFromChipAtIndex',
'announceMessage',
]);
});

const setupTest = () => {
const {foundation, mockAdapter} = setUpFoundationTest(MDCChipSetFoundation);
mockAdapter.hasClass.withArgs(cssClasses.CHOICE).and.returnValue(false);
Expand Down
5 changes: 1 addition & 4 deletions packages/mdc-chips/chip/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export class MDCChipFoundation extends MDCFoundation<MDCChipAdapter> {
};
}


/** Whether a trailing icon click should immediately trigger exit/removal of the chip. */
private shouldRemoveOnTrailingIconClick_ = true;

Expand All @@ -91,10 +92,6 @@ export class MDCChipFoundation extends MDCFoundation<MDCChipAdapter> {
*/
private shouldFocusPrimaryActionOnClick_ = true;

constructor(adapter?: Partial<MDCChipAdapter>) {
super({...MDCChipFoundation.defaultAdapter, ...adapter});
}

isSelected() {
return this.adapter.hasClass(cssClasses.SELECTED);
}
Expand Down
31 changes: 0 additions & 31 deletions packages/mdc-chips/chip/test/foundation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
*/


import {verifyDefaultAdapter} from '../../../../testing/helpers/foundation';
import {setUpFoundationTest, setUpMdcTestEnvironment} from '../../../../testing/helpers/setup';
import {EventSource} from '../constants';
import {MDCChipFoundation} from '../foundation';
Expand All @@ -40,36 +39,6 @@ describe('MDCChipFoundation', () => {
expect('cssClasses' in MDCChipFoundation).toBeTruthy();
});

it('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCChipFoundation, [
'addClass',
'addClassToLeadingIcon',
'eventTargetHasClass',
'focusPrimaryAction',
'focusTrailingAction',
'getAttribute',
'getCheckmarkBoundingClientRect',
'getComputedStyleValue',
'getRootBoundingClientRect',
'hasClass',
'hasLeadingIcon',
'isRTL',
'isTrailingActionNavigable',
'notifyEditFinish',
'notifyEditStart',
'notifyInteraction',
'notifyNavigation',
'notifyRemoval',
'notifySelection',
'notifyTrailingIconInteraction',
'removeClass',
'removeClassFromLeadingIcon',
'removeTrailingActionFocus',
'setPrimaryActionAttr',
'setStyleProperty',
]);
});

const setupTest = () => {
const {foundation, mockAdapter} = setUpFoundationTest(MDCChipFoundation);

Expand Down
4 changes: 0 additions & 4 deletions packages/mdc-chips/trailingaction/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ export class MDCChipTrailingActionFoundation extends
};
}

constructor(adapter?: Partial<MDCChipTrailingActionAdapter>) {
super({...MDCChipTrailingActionFoundation.defaultAdapter, ...adapter});
}

handleClick(evt: MouseEvent) {
evt.stopPropagation();
this.adapter.notifyInteraction(InteractionTrigger.CLICK);
Expand Down
11 changes: 0 additions & 11 deletions packages/mdc-chips/trailingaction/test/foundation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

import {KEY} from '@material/dom/keyboard';

import {verifyDefaultAdapter} from '../../../../testing/helpers/foundation';
import {setUpFoundationTest, setUpMdcTestEnvironment} from '../../../../testing/helpers/setup';
import {InteractionTrigger, strings} from '../constants';
import {MDCChipTrailingActionFoundation} from '../foundation';
Expand All @@ -36,16 +35,6 @@ describe('MDCChipTrailingActionFoundation', () => {
expect('strings' in MDCChipTrailingActionFoundation).toBeTruthy();
});

it('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCChipTrailingActionFoundation, [
'focus',
'getAttribute',
'setAttribute',
'notifyInteraction',
'notifyNavigation',
]);
});

const setupTest = () => {
const {foundation, mockAdapter} =
setUpFoundationTest(MDCChipTrailingActionFoundation);
Expand Down
6 changes: 1 addition & 5 deletions packages/mdc-circular-progress/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ export class MDCCircularProgressFoundation extends
addClass: () => undefined,
getDeterminateCircleAttribute: () => null,
hasClass: () => false,
removeClass: () => undefined,
removeAttribute: () => undefined,
removeClass: () => undefined,
setAttribute: () => undefined,
setDeterminateCircleAttribute: () => undefined,
};
Expand All @@ -54,10 +54,6 @@ export class MDCCircularProgressFoundation extends
private progress_!: number;
private radius_!: number;

constructor(adapter?: Partial<MDCCircularProgressAdapter>) {
super({...MDCCircularProgressFoundation.defaultAdapter, ...adapter});
}

init() {
this.isClosed_ = this.adapter.hasClass(cssClasses.CLOSED_CLASS);
this.isDeterminate_ =
Expand Down
16 changes: 2 additions & 14 deletions packages/mdc-circular-progress/test/foundation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
*/


import {MDCCircularProgressFoundation} from '../../mdc-circular-progress/foundation';
import {checkNumTimesSpyCalledWithArgs, verifyDefaultAdapter} from '../../../testing/helpers/foundation';
import {checkNumTimesSpyCalledWithArgs} from '../../../testing/helpers/foundation';
import {setUpFoundationTest} from '../../../testing/helpers/setup';
import {MDCCircularProgressFoundation} from '../../mdc-circular-progress/foundation';

const {cssClasses, strings} = MDCCircularProgressFoundation;

Expand All @@ -37,18 +37,6 @@ describe('MDCCircularProgressFoundation', () => {
expect('cssClasses' in MDCCircularProgressFoundation).toBeTruthy();
});

it('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCCircularProgressFoundation, [
'addClass',
'getDeterminateCircleAttribute',
'hasClass',
'removeAttribute',
'removeClass',
'setAttribute',
'setDeterminateCircleAttribute',
]);
});

const setupTest = () => {
const {foundation, mockAdapter} =
setUpFoundationTest(MDCCircularProgressFoundation);
Expand Down
5 changes: 1 addition & 4 deletions packages/mdc-data-table/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {SortActionEventData} from './types';
* logic requiring DOM manipulation are delegated to adapter methods.
*/
export class MDCDataTableFoundation extends MDCFoundation<MDCDataTableAdapter> {

static get defaultAdapter(): MDCDataTableAdapter {
return {
addClass: () => undefined,
Expand Down Expand Up @@ -69,10 +70,6 @@ export class MDCDataTableFoundation extends MDCFoundation<MDCDataTableAdapter> {
};
}

constructor(adapter?: Partial<MDCDataTableAdapter>) {
super({...MDCDataTableFoundation.defaultAdapter, ...adapter});
}

/**
* Re-initializes header row checkbox and row checkboxes when selectable rows are added or removed from table.
* Use this if registering checkbox is synchronous.
Expand Down
37 changes: 0 additions & 37 deletions packages/mdc-data-table/test/foundation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,48 +21,11 @@
* THE SOFTWARE.
*/

import {verifyDefaultAdapter} from '../../../testing/helpers/foundation';
import {setUpFoundationTest} from '../../../testing/helpers/setup';
import {attributes, cssClasses, SortValue, strings} from '../constants';
import {MDCDataTableFoundation} from '../foundation';

describe('MDCDataTableFoundation', () => {
it('default adapter returns a complete adapter implementation', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is removing tests in scope of this PR? Can we remove this tests when we delete default adapters?

This may affect code coverage numbers otherwise.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abhiomkar Initially, we removed these tests because we assumed defaultAdapter would no longer be used (since now adapters need to implement the full interface).

But another test mocks an adapter using the defaultAdapter, so just to get the tests to pass, @kseamon suggested putting back the defaultAdapter for now (we can delete them in a later PR)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kseamon Can you please confirm if this is what you intended. http://b/165316767 suggests that unit tests will be cleaned up along with defaultAdapter code.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - missed this way back when - yes, we need to keep the default adaptors around until the tests are refactored.

The most straightforward thing might be to factor them out into objects that live in the test files.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! Moving objects to live in tests SGTM so that the code coverage numbers will be intact.

Since ngwattcos@ left Google, do you want to take a stab at it? You can directly send a CL to us (Copybara will automatically merge the PR).

Let me know if you've any questions!

verifyDefaultAdapter(MDCDataTableFoundation, [
'addClass',
'addClassAtRowIndex',
'getAttributeByHeaderCellIndex',
'getHeaderCellCount',
'getHeaderCellElements',
'getRowCount',
'getRowElements',
'getRowIdAtIndex',
'getRowIndexByChildElement',
'getSelectedRowCount',
'getTableContainerHeight',
'getTableHeaderHeight',
'isCheckboxAtRowIndexChecked',
'isHeaderRowCheckboxChecked',
'isRowsSelectable',
'notifyRowSelectionChanged',
'notifySelectedAll',
'notifySortAction',
'notifyUnselectedAll',
'registerHeaderRowCheckbox',
'registerRowCheckboxes',
'removeClass',
'removeClassAtRowIndex',
'removeClassNameByHeaderCellIndex',
'setAttributeAtRowIndex',
'setAttributeByHeaderCellIndex',
'setClassNameByHeaderCellIndex',
'setHeaderRowCheckboxChecked',
'setHeaderRowCheckboxIndeterminate',
'setProgressIndicatorStyles',
'setRowCheckboxCheckedAtIndex',
'setSortStatusLabelByHeaderCellIndex',
]);
});

function setupTest() {
const {foundation, mockAdapter} =
Expand Down
4 changes: 0 additions & 4 deletions packages/mdc-dialog/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ export class MDCDialogFoundation extends MDCFoundation<MDCDialogAdapter> {
private autoStackButtons_ = true;
private areButtonsStacked_ = false;

constructor(adapter?: Partial<MDCDialogAdapter>) {
super({...MDCDialogFoundation.defaultAdapter, ...adapter});
}

init() {
if (this.adapter.hasClass(cssClasses.STACKED)) {
this.setAutoStackButtons(false);
Expand Down
24 changes: 0 additions & 24 deletions packages/mdc-dialog/test/foundation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
* THE SOFTWARE.
*/

import {verifyDefaultAdapter} from '../../../testing/helpers/foundation';
import {setUpFoundationTest, setUpMdcTestEnvironment} from '../../../testing/helpers/setup';
import {cssClasses, numbers, strings} from '../constants';
import {MDCDialogFoundation} from '../foundation';
Expand All @@ -46,29 +45,6 @@ describe('MDCDialogFoundation', () => {
expect(MDCDialogFoundation.numbers).toEqual(numbers);
});

it('default adapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCDialogFoundation, [
'addClass',
'removeClass',
'hasClass',
'addBodyClass',
'removeBodyClass',
'eventTargetMatches',
'trapFocus',
'releaseFocus',
'getInitialFocusEl',
'isContentScrollable',
'areButtonsStacked',
'getActionFromEvent',
'clickDefaultButton',
'reverseButtons',
'notifyOpening',
'notifyOpened',
'notifyClosing',
'notifyClosed',
]);
});

function setupTest() {
const {foundation, mockAdapter} = setUpFoundationTest(MDCDialogFoundation);
foundation.init();
Expand Down
4 changes: 2 additions & 2 deletions packages/mdc-dom/announce.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ export function announce(message: string, priority?: AnnouncerPriority) {
}

class Announcer {
private static instance: Announcer;
private readonly liveRegions: Map<AnnouncerPriority, Element>;

static getInstance(): Announcer {
if (!Announcer.instance) {
Expand All @@ -52,6 +50,8 @@ class Announcer {

return Announcer.instance;
}
private static instance: Announcer;
private readonly liveRegions: Map<AnnouncerPriority, Element>;

// Constructor made private to ensure only the singleton is used
private constructor() {
Expand Down
Loading