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

refactor(cpe): fixed warnings and type issues in tests #2369

Merged
merged 8 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion packages/control-property-editor-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"scripts": {
"clean": "rimraf --glob dist reports *.tsbuildinfo",
"build": "npm-run-all -l -s clean -p build:compile",
"build:compile": "tsc --build ./tsconfig.json --pretty",
"build:compile": "tsc --build ./tsconfig.json --pretty",
"watch": "build:compile --watch",
"test": "jest --maxWorkers=1 --ci --forceExit --detectOpenHandles --config='jest.config.js'",
"lint": "eslint . --ext .ts,.tsx",
Expand Down
27 changes: 18 additions & 9 deletions packages/control-property-editor-common/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export type ControlProperty =
| IntegerControlProperty
| FloatControlProperty
| StringControlProperty
| ControlPropertyBase<typeof STRING_VALUE_TYPE, string, 'unknown'>
| StringControlPropertyWithOptions;

export interface OutlineNode {
Expand All @@ -114,8 +115,13 @@ export interface IconDetails {
fontFamily: string;
}

export const PENDING_CHANGE_TYPE = 'pending';
export const SAVED_CHANGE_TYPE = 'saved';
export const PROPERTY_CHANGE_KIND = 'property';
export const UNKNOWN_CHANGE_KIND = 'unknown';
export interface PendingPropertyChange<T extends PropertyValue = PropertyValue> extends PropertyChange<T> {
type: 'pending';
type: typeof PENDING_CHANGE_TYPE;
kind: typeof PROPERTY_CHANGE_KIND;
/**
* Indicates if change is before or after current position in undo redo stack
*/
Expand All @@ -124,7 +130,8 @@ export interface PendingPropertyChange<T extends PropertyValue = PropertyValue>
}

export interface PendingOtherChange {
type: 'pending';
type: typeof PENDING_CHANGE_TYPE;
kind: typeof UNKNOWN_CHANGE_KIND;
isActive: boolean;
changeType: string;
controlId: string;
Expand All @@ -133,27 +140,29 @@ export interface PendingOtherChange {
}

export type PendingChange = PendingPropertyChange | PendingOtherChange;
export type SavedChange = SavedPropertyChange | UnknownSavedChange;

export interface SavedPropertyChange<T extends PropertyValue = PropertyValue> extends PropertyChange<T> {
type: 'saved';
kind: 'valid';
type: typeof SAVED_CHANGE_TYPE;
kind: typeof PROPERTY_CHANGE_KIND;
fileName: string;
timestamp: number;
}

export interface UnknownSavedChange {
type: 'saved';
kind: 'unknown';
type: typeof SAVED_CHANGE_TYPE;
kind: typeof UNKNOWN_CHANGE_KIND;
fileName: string;
changeType: string;
controlId?: string;
timestamp?: number;
}
export type ValidChange = PendingPropertyChange | SavedPropertyChange;
export type Change = ValidChange | UnknownSavedChange;

export type Change = PendingChange | SavedChange;

export interface ChangeStackModified {
pending: PendingChange[];
saved: SavedPropertyChange[];
saved: SavedChange[];
}

export interface PropertyChangeDeletionDetails {
Expand Down
4 changes: 2 additions & 2 deletions packages/control-property-editor-common/src/debounce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* @param delay Idle period in milliseconds after which the callback will be executed
* @returns A wrapper function that should be called to invoke the callback function after delay
*/
export function debounce(callback: Function, delay: number): (args?: unknown[]) => void {
export function debounce<T extends []>(callback: (...args: T) => void, delay: number): (...args: T) => void {
let timerId: NodeJS.Timeout;
return (...args): void => {
return (...args: T): void => {
clearTimeout(timerId);

timerId = setTimeout(() => {
Expand Down
13 changes: 9 additions & 4 deletions packages/control-property-editor-common/test/unit/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,18 @@ describe('createExternalAction', () => {
});

test('changeStackModified', () => {
const payload = {
const payload: ChangeStackModified = {
pending: [
{
controlId: 'testPendingId',
isActive: true,
propertyName: 'testPendingProp',
type: 'pending',
value: 'test',
controlName: 'test'
controlName: 'test',
kind: 'property',
changeType: 'propertyChange',
fileName: 'fileName1'
}
],
saved: [
Expand All @@ -133,11 +136,13 @@ describe('createExternalAction', () => {
type: 'saved',
value: 'test',
fileName: 'testSaveId.change',
kind: 'valid',
kind: 'property',
changeType: 'propertyChange',
controlName: 'button',
timestamp: 12343310032023
}
]
} as ChangeStackModified;
};
const changedProp = changeStackModified(payload);
expect(changedProp.type).toBe('[ext] change-stack-modified');
expect(changedProp.payload).toStrictEqual(payload);
Expand Down
1 change: 1 addition & 0 deletions packages/control-property-editor/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ config.globals = {
config.preset = 'ts-jest';
config.transformIgnorePatterns = ['<rootDir>/node_modules/'];
config.testMatch = ['**/test/unit/**/*.(test).ts(x)?'];
config.setupFiles = ['./test/unit/setup.ts'];
module.exports = config;
2 changes: 1 addition & 1 deletion packages/control-property-editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"scripts": {
"clean": "rimraf --glob dist coverage *.tsbuildinfo",
"build": "npm-run-all -l -s clean -p build:compile build:webapp",
"build:compile": "tsc --build ./tsconfig.json --pretty --dry",
"build:compile": "tsc --noEmit",
"build:webapp": "node esbuild.js",
"watch": "npm-run-all -p watch:*",
"watch:webapp": "node esbuild.js --watch --minify=false",
Expand Down
1 change: 0 additions & 1 deletion packages/control-property-editor/src/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,5 @@ export function initI18n(language = 'en'): void {
escapeValue: false
}
})
.then(() => console.log('i18n initialized'))
.catch((error) => console.error(error));
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
import type { ReactElement } from 'react';
import React from 'react';
import { Stack } from '@fluentui/react';
import type { Change, ValidChange } from '@sap-ux-private/control-property-editor-common';
import type { Change } from '@sap-ux-private/control-property-editor-common';
import {
convertCamelCaseToPascalCase,
PENDING_CHANGE_TYPE,
PROPERTY_CHANGE_KIND,
SAVED_CHANGE_TYPE,
UNKNOWN_CHANGE_KIND
} from '@sap-ux-private/control-property-editor-common';

import { Separator } from '../../components';
import type { ControlGroupProps, ControlChange } from './ControlGroup';
import type { ControlGroupProps } from './ControlGroup';
import { ControlGroup } from './ControlGroup';
import type { UnknownChangeProps } from './UnknownChange';
import { UnknownChange } from './UnknownChange';
Expand All @@ -14,7 +21,6 @@ import { useSelector } from 'react-redux';
import type { FilterOptions } from '../../slice';
import { FilterName } from '../../slice';
import type { RootState } from '../../store';
import { convertCamelCaseToPascalCase } from '@sap-ux-private/control-property-editor-common';
import { getFormattedDateAndTime } from './utils';

export interface ChangeStackProps {
Expand All @@ -35,14 +41,14 @@ export function ChangeStack(changeStackProps: ChangeStackProps): ReactElement {
.value.toString()
.toLowerCase();
groups = filterGroup(groups, filterQuery);
const stackName = changes[0].type === 'pending' ? 'unsaved-changes-stack' : 'saved-changes-stack';
const stackName = changes[0].type === PENDING_CHANGE_TYPE ? 'unsaved-changes-stack' : 'saved-changes-stack';
return (
<Stack data-testid={stackName} tokens={{ childrenGap: 5, padding: '5px 0px 5px 0px' }}>
{groups.map((item, i) => [
isKnownChange(item) ? (
isControlGroup(item) ? (
<Stack.Item
data-testid={`${stackName}-${item.controlId}-${item.changeIndex}`}
key={`${item.controlId}-${item.changeIndex}`}>
data-testid={`${stackName}-${item.controlId}-${item.index}`}
key={`${item.controlId}-${item.index}`}>
<ControlGroup {...item} />
</Stack.Item>
) : (
Expand Down Expand Up @@ -87,7 +93,7 @@ function convertChanges(changes: Change[]): Item[] {
while (i < changes.length) {
const change: Change = changes[i];
let group: ControlGroupProps;
if (change.type === 'saved' && change.kind === 'unknown') {
if (change.kind === UNKNOWN_CHANGE_KIND && change.type === SAVED_CHANGE_TYPE) {
items.push({
fileName: change.fileName,
timestamp: change.timestamp,
Expand All @@ -100,21 +106,18 @@ function convertChanges(changes: Change[]): Item[] {
controlId: change.controlId,
controlName: change.controlName,
text: convertCamelCaseToPascalCase(change.controlName),
changeIndex: i,
changes: [classifyChange(change, i)]
index: i,
changes: [change]
};
items.push(group);
i++;
while (i < changes.length) {
// We don't need to add header again if the next control is the same
const nextChange = changes[i];
if (
(nextChange.type === 'saved' && nextChange.kind === 'unknown') ||
change.controlId !== nextChange.controlId
) {
if (nextChange.kind === UNKNOWN_CHANGE_KIND || change.controlId !== nextChange.controlId) {
break;
}
group.changes.push(classifyChange(nextChange, i));
group.changes.push(nextChange);
i++;
}
}
Expand All @@ -123,72 +126,28 @@ function convertChanges(changes: Change[]): Item[] {
}

/**
* Classify Change for grouping.
* Checks if item is of type {@link ControlGroupProps}.
*
* @param change ValidChange
* @param changeIndex number
* @returns ControlChange
*/
function classifyChange(change: ValidChange, changeIndex: number): ControlChange {
let base;
if (change.changeType === 'propertyChange' || change.changeType === 'propertyBindingChange') {
const { controlId, propertyName, value, controlName, changeType } = change;
base = {
controlId,
controlName,
propertyName,
value,
changeIndex,
changeType
};
} else {
const { controlId, controlName, changeType } = change;
base = {
controlId,
controlName,
changeIndex,
changeType
};
}
if (change.type === 'pending') {
const { isActive } = change;
return {
...base,
isActive
};
} else {
const { fileName, timestamp } = change;
return {
...base,
isActive: true,
fileName,
timestamp
};
}
}

/**
* Returns true, if controlName is defined.
*
* @param change ControlGroupProps | UnknownChangeProps
* @param item ControlGroupProps | UnknownChangeProps
* @returns boolean
*/
export function isKnownChange(change: ControlGroupProps | UnknownChangeProps): change is ControlGroupProps {
return (change as ControlGroupProps).controlName !== undefined;
function isControlGroup(item: ControlGroupProps | UnknownChangeProps): item is ControlGroupProps {
return (item as ControlGroupProps).controlName !== undefined;
}

const filterPropertyChanges = (changes: ControlChange[], query: string): ControlChange[] => {
return changes.filter((item) => {
if (item.propertyName) {
const filterPropertyChanges = (changes: Change[], query: string): Change[] => {
return changes.filter((item): boolean => {
if (item.kind === PROPERTY_CHANGE_KIND) {
return (
!query ||
item.propertyName.trim().toLowerCase().includes(query) ||
convertCamelCaseToPascalCase(item.propertyName.toString()).trim().toLowerCase().includes(query) ||
item.value.toString().trim().toLowerCase().includes(query) ||
convertCamelCaseToPascalCase(item.value.toString()).trim().toLowerCase().includes(query) ||
(item.timestamp && getFormattedDateAndTime(item.timestamp).trim().toLowerCase().includes(query))
(item.type === SAVED_CHANGE_TYPE &&
getFormattedDateAndTime(item.timestamp).trim().toLowerCase().includes(query))
);
} else if (item.changeType) {
} else {
const changeType = convertCamelCaseToPascalCase(item.changeType);
return !query || changeType.trim().toLowerCase().includes(query);
}
Expand Down Expand Up @@ -225,7 +184,7 @@ function filterGroup(model: Item[], query: string): Item[] {
}
for (const item of model) {
let parentMatch = false;
if (!isKnownChange(item)) {
if (!isControlGroup(item)) {
if (isQueryMatchesChange(item, query)) {
filteredModel.push({ ...item, changes: [] });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,6 @@ import styles from './ChangesPanel.module.scss';
import { FileChange } from './FileChange';
import { defaultFontSize } from '../properties/constants';

export interface ChangeProps {
controlId: string;
controlName: string;
changeIndex: number;
changeType: string;
propertyName: string;
value: string | number | boolean;
isActive: boolean;
timestamp?: number;
fileName?: string;
/**
* Class used for showing and hiding actions
*/
actionClassName: string;
}

/**
* React element for ChangePanel.
*
Expand Down Expand Up @@ -70,7 +54,7 @@ export function ChangesPanel(): ReactElement {
<Separator />
<Icon iconName="Info" title={fileChangesTooltip} className={styles.infoIcon} />
<ChangeStackHeader
backgroundColor="var(--vscode-sideBar-background);"
backgroundColor="var(--vscode-sideBar-background)"
color="var(--vscode-editor-foreground)"
fontSize={defaultFontSize}
tooltip={fileChangesTooltip}
Expand All @@ -85,7 +69,7 @@ export function ChangesPanel(): ReactElement {
<>
<Separator />
<ChangeStackHeader
backgroundColor="var(--vscode-sideBar-background);"
backgroundColor="var(--vscode-sideBar-background)"
color="var(--vscode-editor-foreground)"
text={t('CHANGE_SUMMARY_UNSAVED_CHANGES')}
/>
Expand All @@ -97,7 +81,7 @@ export function ChangesPanel(): ReactElement {
<>
<Separator />
<ChangeStackHeader
backgroundColor="var(--vscode-sideBar-background);"
backgroundColor="var(--vscode-sideBar-background)"
color="var(--vscode-terminal-ansiGreen)"
text={t('CHANGE_SUMMARY_SAVED_CHANGES')}
/>
Expand Down
Loading
Loading