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

🐞 反馈问题:部分情况下的元素异常遮挡问题 #193

Closed
4 tasks done
Kyle-Ye opened this issue Oct 26, 2023 · 10 comments
Closed
4 tasks done

🐞 反馈问题:部分情况下的元素异常遮挡问题 #193

Kyle-Ye opened this issue Oct 26, 2023 · 10 comments
Labels
bug Something isn't working fixed in next release The issue will be closed once next release is available

Comments

@Kyle-Ye
Copy link
Collaborator

Kyle-Ye commented Oct 26, 2023

请先确认以下事项:

  • 已仔细阅读了 README
  • issues 页面搜索过问题(包括已关闭的 issue),但未能找到解决方法
  • Easydict 已升级到最新版本

问题描述

image image

重现步骤

  1. 仅使用苹果翻译
  2. 选择合适长度的语句进行翻译
  3. 系统设置 - 通用 - 显示滚动条 - 始终
image

期望结果

UI 正常显示(功能按钮位于下方而非遮挡)

解决方案(可选)

No response

设备信息 && 操作系统版本

No response

是否愿意提交 PR 解决该问题?

  • 我愿意提交 PR!
@Kyle-Ye Kyle-Ye added the bug Something isn't working label Oct 26, 2023
@github-actions
Copy link

Hello Kyle-Ye, Thank you for your first issue contribution 🎉

@Kyle-Ye
Copy link
Collaborator Author

Kyle-Ye commented Oct 26, 2023

image

计算高度时使用的宽度是 324,但最终生效约束下的宽度为 313,导致了布局异常。

- (CGSize)labelSize:(EZLabel *)label exceptedWidth:(CGFloat)exceptedWidth {
// ???: 很奇怪,比如实际计算结果为 364,但界面渲染却是 364.5 😑
NSWindow *window = [self windowOfType:self.result.service.windowType];
CGFloat selfWidth = window ? window.width - EZHorizontalCellSpacing_10 * 2 : self.width;
CGFloat width = selfWidth - exceptedWidth;
// NSLog(@"text: %@, width: %@", label.text, @(width));
// NSLog(@"self.width: %@, selfWidth: %@", @(self.width), @(selfWidth));
CGFloat height = [label ez_getTextViewHeightDesignatedWidth:width]; // 397 ?
// NSLog(@"height: %@", @(height));
return CGSizeMake(width, height);
}

@Kyle-Ye
Copy link
Collaborator Author

Kyle-Ye commented Oct 26, 2023

宽度计算逻辑和实际不匹配导致,某次更新中为外层的 NSScrollView 添加了 verticalScroller,导致宽度减少了11

scrollView.hasVerticalScroller = YES;

修复方案建议:

  1. 关闭垂直滚动条
// EZBaseQueryViewController.m
- scrollView.hasVerticalScroller = YES;
+ scrollView.hasVerticalScroller = NO;

优点:滚动条的加入不仅导致了这里宽度计算的不一致异常,还破坏了 UI 对齐的美感 (Chrome 图标和左边的置顶图标在README的截图中显示在没有滚动条的情况下是和文本框的两侧对齐的)
缺点:在含有多种翻译方式的情况下缺少滚动条不利于用户交互

image
  1. 更新宽度计算逻辑
    获取当前容器宽度而非 Window 宽度或者在当前的宽度计算实现中加入滚动条宽度的考虑
// EZWordResultView.m
- NSWindow *window = [self windowOfType:self.result.service.windowType];
- CGFloat selfWidth = window ? window.width - EZHorizontalCellSpacing_10 * 2 : self.width;
- CGFloat width = selfWidth - exceptedWidth;
+ CGFloat width = self.width - exceptedWidth;
image

@tisfeng 作者可以帮忙评估下上下文完成决策后我再提相关 PR 吧

@tisfeng
Copy link
Owner

tisfeng commented Oct 27, 2023

还真是,没想到还有个始终显示滚动条情况,这个高度计算我修改调试过很多次了,难怪之前没有发现 😓

@tisfeng
Copy link
Owner

tisfeng commented Oct 27, 2023

回顾了一下这里的改动 a282429 ,最开始就是

CGFloat width = self.width - exceptedWidth;

看提交日志,好像后面发现有问题才改的,但我刚回退之前的代码,测试了一下好像也没有问题。。

fix(UI): calculate EZLabel width incorrectly when resizing window suddenly

那暂时就先改回来,观察一段时间看看。

@Kyle-Ye
Copy link
Collaborator Author

Kyle-Ye commented Oct 27, 2023

那暂时就先改回来,观察一段时间看看。

或者有考虑后续改为 SwiftUI 这类 DSL 方案吗?😀 See #194

@tisfeng
Copy link
Owner

tisfeng commented Oct 27, 2023

SwiftUI 也可以,但我很久没写这个了,只在之前刚出的时候写过一点,当时遇到了很多坑,觉得还不成熟,就搁置了。

如果你认为目前 SwiftUI 已经比较成熟了,那你可以尝试先用它在项目中写一些新的页面,我们先观察、适应一下,看看实际效果如何。

@Kyle-Ye
Copy link
Collaborator Author

Kyle-Ye commented Oct 27, 2023

SwiftUI 也可以,但我很久没写这个了,只在之前刚出的时候写过一点,当时遇到了很多坑,觉得还不成熟,就搁置了。

如果你认为目前 SwiftUI 已经比较成熟了,那你可以尝试先用它在项目中写一些新的页面,我们先观察、适应一下,看看实际效果如何。

iOS 上的 SwiftUI 基本已经足够成熟可以完成完整项目(iOS 16+),macOS 上的 SwiftUI 仍有部分已知 Bug,完整工程使用需谨慎。
https://github.com/feedback-assistant/reports/issues

但是如果不是整个 App 100% SwiftUI 化,而是部分界面/UI元素使用 SwiftUI 是没啥问题的。

老 OS 上的各种 SwiftUI bug 如果熟悉内部原理,也基本都有方案可以绕过。

使用 Swift & SwiftUI 的还有一个副作用是可能会加速工程提高 Target 的速度,因为部分 API 兼容成本比较高

@tisfeng tisfeng added the fixed in next release The issue will be closed once next release is available label Oct 28, 2023
@tisfeng
Copy link
Owner

tisfeng commented Oct 28, 2023

暂时先保持 macOS 11.0+ 的 Target,之前看到有部份用户还在这个系统版本。

稍后我统计一下具体的用户数量,如果不多的话,可以考虑提到 macOS 12,我们大约保持最新版本 -2 就行。

@tisfeng
Copy link
Owner

tisfeng commented Nov 16, 2023

2.0.2 版本已修复该问题。

@tisfeng tisfeng closed this as completed Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in next release The issue will be closed once next release is available
Projects
None yet
Development

No branches or pull requests

2 participants