Skip to content

Commit

Permalink
fix: 修复 React 18 环境下 Tooltip 卸载后无法再次渲染 & 排序菜单选中效果丢失的问题 (#2698)
Browse files Browse the repository at this point in the history
* fix: 修复 React 18 环境下 Tooltip 卸载后无法再次渲染 & 排序菜单选中效果丢失的问题

* test: 单测修复

* test: 单测修复

---------

Co-authored-by: Wenjun Xu <[email protected]>
  • Loading branch information
lijinke666 and wjgogogo authored May 9, 2024
1 parent 98c051a commit 0af329d
Show file tree
Hide file tree
Showing 20 changed files with 104 additions and 158 deletions.
13 changes: 1 addition & 12 deletions packages/s2-core/__tests__/unit/sheet-type/pivot-sheet-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,6 @@ describe('PivotSheet Tests', () => {

const options: TooltipOptions = {
onlyShowOperator: true,
forceRender: true,
operator: {
menu: {
items: [
Expand All @@ -1014,7 +1013,7 @@ describe('PivotSheet Tests', () => {
{ key: 'none', label: groupNoneText },
],
onClick: expect.anything(),
defaultSelectedKeys: [],
selectedKeys: [],
},
},
};
Expand All @@ -1033,14 +1032,6 @@ describe('PivotSheet Tests', () => {
.spyOn(s2, 'render')
.mockImplementation(async () => {});

const showTooltipWithInfoSpy = jest
.spyOn(s2, 'showTooltipWithInfo')
.mockImplementation((_, __, options) => {
return {
forceRender: options?.forceRender,
} as unknown as void;
});

const nodeMeta = new Node({ id: '1', field: '1', value: 'testValue' });

s2.handleGroupSort(
Expand All @@ -1050,8 +1041,6 @@ describe('PivotSheet Tests', () => {
nodeMeta,
);

expect(showTooltipWithInfoSpy).toHaveReturnedWith({ forceRender: true });

s2.groupSortByMethod('asc', nodeMeta);

expect(s2.dataCfg.sortParams).toEqual([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ describe('TableSheet Tests', () => {
{
operator: expect.anything(),
onlyShowOperator: true,
forceRender: true,
},
);

Expand Down Expand Up @@ -191,7 +190,6 @@ describe('TableSheet Tests', () => {

const options: TooltipOptions = {
onlyShowOperator: true,
forceRender: true,
operator: {
menu: {
items: [
Expand All @@ -200,7 +198,7 @@ describe('TableSheet Tests', () => {
{ key: 'none', label: groupNoneText },
],
onClick: expect.anything(),
defaultSelectedKeys: [],
selectedKeys: [],
},
},
};
Expand Down
31 changes: 22 additions & 9 deletions packages/s2-core/scripts/test-live.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,34 @@ import { default as autoCompletePrompt } from 'inquirer-autocomplete-prompt';

inquirer.registerPrompt('autocomplete', autoCompletePrompt);

function run(path) {
const command = `cross-env DEBUG_MODE=1 npx jest ${path}`;
const jestSpinner = ora(`[测试运行中]: ${command}`).start();

try {
execSync(command);
jestSpinner.succeed('测试运行完成.');
} catch (error) {
jestSpinner.fail();
}
}

async function main() {
const spinner = ora('读取测试文件中...').start();
const paths = glob.sync(`!(node_modules)/**/*-spec.ts?(x)`);

const defaultPath = '__tests__/spreadsheet/spread-sheet-spec.ts';
const customPath = process.argv[2];
const defaultPath =
customPath || '__tests__/spreadsheet/spread-sheet-spec.tsx';

spinner.stop();

if (customPath) {
run(customPath);

return;
}

const selectedPath = await inquirer.prompt([
{
type: 'autocomplete',
Expand All @@ -34,14 +54,7 @@ async function main() {
},
]);

const jestSpinner = ora('测试运行中...').start();

try {
execSync(`cross-env DEBUG_MODE=1 npx jest ${selectedPath.path}`);
jestSpinner.succeed('测试运行完成.');
} catch (error) {
jestSpinner.fail();
}
run(selectedPath.path);
}

main();
4 changes: 2 additions & 2 deletions packages/s2-core/src/common/interface/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ export interface TooltipOperatorMenuOptions<Icon, Text> {
onClick?: TooltipOperatorClickHandler;

/**
* 默认选中的菜单项 key
* 选中的菜单项 key
*/
defaultSelectedKeys?: string[];
selectedKeys?: string[];
}

export interface TooltipOperatorOptions<Menu = BaseTooltipOperatorMenuOptions> {
Expand Down
23 changes: 6 additions & 17 deletions packages/s2-core/src/sheet-type/spread-sheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
} from '@antv/g';
import { Renderer } from '@antv/g-canvas';
import {
delay,
forEach,
forIn,
get,
Expand Down Expand Up @@ -61,16 +60,15 @@ import type { BaseDataSet } from '../data-set';
import type { BaseFacet } from '../facet';
import type { Node } from '../facet/layout/node';
import { RootInteraction } from '../interaction/root';
import { getTheme } from '../theme';
import { HdAdapter } from '../ui/hd-adapter';
import { BaseTooltip } from '../ui/tooltip';
import { removeOffscreenCanvas } from '../utils/canvas';
import { clearValueRangeState } from '../utils/condition/state-controller';
import { hideColumnsByThunkGroup } from '../utils/hide-columns';
import { isMobile } from '../utils/is-mobile';
import { customMerge, setupDataConfig, setupOptions } from '../utils/merge';
import { injectThemeVars } from '../utils/theme';
import { getTooltipData, getTooltipOptions } from '../utils/tooltip';
import { getTheme } from '../theme';

export abstract class SpreadSheet extends EE {
public themeName: ThemeName;
Expand Down Expand Up @@ -272,14 +270,7 @@ export abstract class SpreadSheet extends EE {
onMounted: resolve,
};

if (isMobile()) {
// S2 的在点击会触发两次,一次在 Canvas 上,一次会在 Drawer mask 上。
delay(() => {
this.tooltip.show?.(options);
}, 50);
} else {
this.tooltip.show?.(options);
}
this.tooltip?.show?.(options);
});
}

Expand Down Expand Up @@ -320,11 +311,11 @@ export abstract class SpreadSheet extends EE {
}

public hideTooltip() {
this.tooltip.hide?.();
this.tooltip?.hide?.();
}

public destroyTooltip() {
this.tooltip.destroy?.();
this.tooltip?.destroy?.();
}

public registerIcons() {
Expand Down Expand Up @@ -897,14 +888,14 @@ export abstract class SpreadSheet extends EE {
event.stopPropagation();
this.interaction.addIntercepts([InterceptType.HOVER]);

const defaultSelectedKeys = this.getMenuDefaultSelectedKeys(meta?.id);
const selectedKeys = this.getMenuDefaultSelectedKeys(meta?.id);
const menuItems: TooltipOperatorMenuItems = this.isTableMode()
? getTooltipOperatorTableSortMenus()
: getTooltipOperatorSortMenus();

const operator: TooltipOperatorOptions = {
menu: {
defaultSelectedKeys,
selectedKeys,
items: menuItems,
onClick: ({ key: sortMethod }) => {
this.groupSortByMethod(sortMethod as SortMethod, meta);
Expand All @@ -916,8 +907,6 @@ export abstract class SpreadSheet extends EE {
this.showTooltipWithInfo(event, [], {
operator,
onlyShowOperator: true,
// 确保 tooltip 内容更新 https://github.com/antvis/S2/issues/1716
forceRender: true,
});
}
}
1 change: 1 addition & 0 deletions packages/s2-core/src/styles/theme/dark.less
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
@{css-var-prefix}-background: #000;
@{css-var-prefix}-tooltip-background: rgba(43, 43, 43, 0.95);
@{css-var-prefix}-tooltip-operator-background: rgba(43, 43, 43, 0.95);
@{css-var-prefix}-tooltip-operator-menu-selected-background: #1677ff;
}
4 changes: 3 additions & 1 deletion packages/s2-core/src/utils/export/copy/table-copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ class TableDataCellCopy extends BaseDataCellCopy {
: rowLength;

while (
(deadline.timeRemaining() > 0 || deadline.didTimeout) &&
(deadline.timeRemaining() > 0 ||
deadline.didTimeout ||
process.env['NODE_ENV'] === 'test') &&
rowIndex <= rowLength - 1 &&
count > 0
) {
Expand Down
2 changes: 1 addition & 1 deletion packages/s2-core/src/utils/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export const getTooltipDefaultOptions = <Menu = BaseTooltipOperatorMenuOptions>(
menu: {
onClick: noop,
items: [],
defaultSelectedKeys: [],
selectedKeys: [],
},
},
enableFormat: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Tooltip Component Tests', () => {
menu: {
items:
getTooltipOperatorSortMenus() as TooltipOperatorMenuItems,
defaultSelectedKeys: [key],
selectedKeys: [key],
},
},
}}
Expand Down Expand Up @@ -68,7 +68,7 @@ describe('Tooltip Common Components Tests', () => {
menu={{
items: menus,
onClick: mockMenuClick,
defaultSelectedKeys: [menus[0].key],
selectedKeys: [menus[0].key],
}}
key={'tooltipOperator'}
cell={mockCell as unknown as S2CellType}
Expand Down
2 changes: 1 addition & 1 deletion packages/s2-react/playground/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ function MainLayout() {
}

return (
s2Ref.current?.facet.getInitColLeafNodes().map(({ id }) => id) || []
s2Ref.current?.facet?.getInitColLeafNodes().map(({ id }) => id) || []
);
},
[dataCfg.fields?.columns],
Expand Down
31 changes: 22 additions & 9 deletions packages/s2-react/scripts/test-live.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,34 @@ import { default as autoCompletePrompt } from 'inquirer-autocomplete-prompt';

inquirer.registerPrompt('autocomplete', autoCompletePrompt);

function run(path) {
const command = `cross-env DEBUG_MODE=1 npx jest ${path}`;
const jestSpinner = ora(`[测试运行中]: ${command}`).start();

try {
execSync(command);
jestSpinner.succeed('测试运行完成.');
} catch (error) {
jestSpinner.fail();
}
}

async function main() {
const spinner = ora('读取测试文件中...').start();
const paths = glob.sync(`!(node_modules)/**/*-spec.ts?(x)`);

const defaultPath = '__tests__/spreadsheet/spread-sheet-spec.tsx';
const customPath = process.argv[2];
const defaultPath =
customPath || '__tests__/spreadsheet/spread-sheet-spec.tsx';

spinner.stop();

if (customPath) {
run(customPath);

return;
}

const selectedPath = await inquirer.prompt([
{
type: 'autocomplete',
Expand All @@ -35,14 +55,7 @@ async function main() {
},
]);

const jestSpinner = ora('测试运行中...').start();

try {
execSync(`cross-env DEBUG_MODE=1 npx jest ${selectedPath.path}`);
jestSpinner.succeed('测试运行完成.');
} catch (error) {
jestSpinner.fail();
}
run(selectedPath.path);
}

main();
2 changes: 0 additions & 2 deletions packages/s2-react/src/common/constant/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import type { SpreadSheet } from '@antv/s2';
import type { SheetComponentOptions } from '../../components';
import { CustomTooltip } from '../../components/tooltip/custom-tooltip';

export const MOBILE_DRAWER_WIDTH = 300;

export const RENDER_TOOLTIP_OPTIONS: SheetComponentOptions = {
tooltip: {
render: (spreadsheet: SpreadSheet) => new CustomTooltip(spreadsheet),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const TooltipOperator: React.FC<Required<TooltipOperatorProps>> =
className,
items: menus,
onClick,
defaultSelectedKeys,
selectedKeys,
...otherMenuProps
},
} = props;
Expand Down Expand Up @@ -65,7 +65,7 @@ export const TooltipOperator: React.FC<Required<TooltipOperatorProps>> =
mode={onlyShowOperator ? 'vertical' : 'horizontal'}
className={cls(`${TOOLTIP_PREFIX_CLS}-operator-menus`, className)}
onClick={onMenuClick}
defaultSelectedKeys={defaultSelectedKeys}
selectedKeys={selectedKeys}
items={items}
selectable={onlyShowOperator}
{...otherMenuProps}
Expand Down
Loading

0 comments on commit 0af329d

Please sign in to comment.