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.

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

fix: 修复 Popover.Confirm 的弹出容器的宽度在Table中有可能显示较窄的问题 #736

Merged
merged 2 commits into from
Oct 18, 2024
Merged

fix: 修复 Popover.Confirm 的弹出容器的宽度在Table中有可能显示较窄的问题 #736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50a9216
fix: 修复 `Popover.Confirm` 的弹出容器的宽度在Table中有可能显示较窄的问题
Oct 17, 2024
87c3534
fix: fix textarea test-case
Oct 17, 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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "sheinx",
"private": true,
"version": "3.4.4-beta.7",
"version": "3.4.4-beta.8",
"description": "A react library developed with sheinx",
"module": "dist/index.js",
"types": "dist/index.d.ts",
Expand Down
29 changes: 15 additions & 14 deletions packages/hooks/src/utils/position.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,23 @@ const getPopoverPosition = (
if (verticalPoint > windowHeight / 2) position = 'top';
else position = 'bottom';

// TODO: 暂时移除，另考虑方案
// 如果渲染了弹出内容，则根据弹出内容宽度计算是否自动调整位置
if (popupRect) {
if (popupRect?.width / 2 > rect.left) {
position += '-left';
}
if (popupRect?.width / 2 > windowWidth - rect.right) {
position += '-right';
}
} else {
// 兜底计算
if (horizontalPoint > windowWidth * 0.6) {
position += '-right';
} else if (horizontalPoint < windowWidth * 0.4) {
position += '-left';
}
// if (popupRect && popupRect?.width) {
// if (popupRect?.width / 2 > rect.left) {
// position += '-left';
// }
// if (popupRect?.width / 2 > windowWidth - rect.right) {
// position += '-right';
// }
// } else {
// 兜底计算
if (horizontalPoint > windowWidth * 0.6) {
position += '-right';
} else if (horizontalPoint < windowWidth * 0.4) {
position += '-left';
}
// }
Comment on lines +78 to +94
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

需要重新考虑弹出框定位逻辑

  1. 移除基于弹出内容宽度的定位逻辑可能会影响某些场景下弹出框的准确定位。特别是在弹出内容较宽的情况下，可能会导致显示不完整或位置不合理的问题。

  2. TODO 注释表明正在考虑替代方案。建议尽快确定新的解决方案，以确保弹出框在各种情况下都能正确定位。在实现新方案之前，可以考虑添加一个 FIXME 注释，以便跟踪这个待解决的问题。

  3. 当前的兜底计算逻辑相对简单，可能无法处理所有用例。建议增强fallback逻辑，考虑更多因素，如：

    • 弹出内容的预估宽度
    • 目标元素的大小和位置
    • 可用的屏幕空间

建议：

  1. 制定清晰的计划来解决这个定位问题，可能需要重新设计定位算法。
  2. 在实现新方案之前，可以添加日志或警告，以便在生产环境中监控可能出现的定位问题。
  3. 考虑添加配置选项，允许用户在需要时手动指定弹出框的首选位置。

您是否需要我协助设计新的定位算法或创建相关的GitHub issue来跟踪这个问题？

}
return position as PopoverPosition;
};
Expand Down
7 changes: 7 additions & 0 deletions packages/shineout/src/popover/__doc__/changelog.cn.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 3.4.4-beta.8
2024-10-17

### 🐞 BugFix
- 修复 `Popover.Confirm` 的弹出容器的宽度在Table中有可能显示较窄的问题 ([#736](https://github.com/sheinsight/shineout-next/pull/736))


## 3.4.3
2024-10-14

Expand Down
15 changes: 8 additions & 7 deletions packages/shineout/src/textarea/__test__/textarea.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const {
wrapperSmall: textareaSmallClassName,
shadow: textareaShadowClassName,
info: textareaInfoClassName,
infoError: textareaInfoErrorClassName,
footer: textareaFooterClassName,
wrapperUnderline: textareaUnderlineClassName,
wrapperNoBorder: textareaNoBorderClassName,
Expand Down Expand Up @@ -152,7 +151,7 @@ describe('Textarea[Info:function]', () => {
await waitFor(async () => {
await delay(200);
});
classLengthTest(container, textareaInfoClassName, 1);
expect(document.querySelectorAll(textareaInfoClassName).length).toBe(1);
});
test('should render tip we want', () => {
const { container } = render(<TextareaInfoFunction />);
Expand All @@ -161,11 +160,13 @@ describe('Textarea[Info:function]', () => {
fireEvent.change(textareaWrapper, {
target: { value: 'test' },
});
textContentTest(container.querySelector(textareaInfoClassName)!, 'total is 4');
// 示例中把提示信息render到body上了，所以这里需要document.querySelector
textContentTest(document.querySelector(textareaInfoClassName)!, 'total is 4');
fireEvent.change(textareaWrapper, {
target: { value: 'testtesttesttesttesttest' },
});
classTest(container.querySelector(textareaInfoClassName)!, textareaInfoErrorClassName);
// 错误信息时，期望container下没有textareaInfoClassName的元素
expect(document.querySelectorAll(textareaInfoClassName).length).toBe(0);
});
});
describe('Textarea[Info:number]', () => {
Expand All @@ -178,8 +179,7 @@ describe('Textarea[Info:number]', () => {
test('should render when not set value', () => {
const { container } = render(<Textarea info={20} />);
expect(
container.querySelectorAll(textareaInfoClassName).length ||
container.querySelectorAll('.' + textareaInfoErrorClassName).length,
container.querySelectorAll(textareaInfoClassName).length
).toBe(0);
});
test('should render tip', () => {
Expand All @@ -196,7 +196,8 @@ describe('Textarea[Info:number]', () => {
fireEvent.change(container.querySelector('textarea') as HTMLTextAreaElement, {
target: { value: '1234456789012345678901' },
});
textContentTest(container.querySelector('.' + textareaInfoErrorClassName)!, '22 / 20');
const selector = `.soui-popover-wrapper[data-soui-type="error"] .soui-popover-text`
textContentTest(container.querySelector(selector)!, '22 / 20');
});
});
describe('Textarea[Trim]', () => {
Expand Down