Skip to content

对V4.2.1的UI修改PR#88

Closed
Huang970 wants to merge 7 commits into
jamesphotography:masterfrom
Huang970:master
Closed

对V4.2.1的UI修改PR#88
Huang970 wants to merge 7 commits into
jamesphotography:masterfrom
Huang970:master

Conversation

@Huang970
Copy link
Copy Markdown

@Huang970 Huang970 commented May 3, 2026

于老师您好,
按照自己需要围绕UI功能的易用性做的一点修改,是否有合并价值,请您判断。
修改如下:
结果浏览画面:
1,结果浏览画面,增加收藏,利用4星以上代表用户选择的收藏。可直接点击❤️操作。
2,连拍/单张筛选开关,展开全部连拍开关
3,增加导出按钮,支持单选/多选/全选照片(CTRL+A),复制所选照片到制定目录。
4,修正低分照片不存在裁切图场合的不刷新问题。
全屏显示画面:
1,增加比率截图功能,CTRL+C到剪辑版(右键菜单)和保存按钮
2,连拍组内翻页按钮调整至上部
两图比较画面:
1,增加焦点显示
2,画面上部布局微调
全体:
按钮式样做了一些微调。

另外V4.2.1的一些UI的bug顺带修改了一些,但记不清楚具体有哪些了。

@jamesphotography
Copy link
Copy Markdown
Owner

Code review

Found 7 issues:

  1. 跨平台破坏:import winreg 无条件加载,macOS 上模块导入即抛 ModuleNotFoundError,整个 results browser 无法打开 (CLAUDE.md 要求 "Keep changes cross-platform (Windows + macOS).")

import glob
import shutil
import winreg
from collections import Counter
from datetime import datetime

  1. 跨平台破坏:_default_image_viewer 使用 Windows 专属的 cmd /c startsubprocess.CREATE_NO_WINDOW,无 sys.platform 分支;macOS 上点击右键菜单"用 X 打开"会 AttributeError (CLAUDE.md 要求 "Keep changes cross-platform")

return "默认程序"
def _default_image_viewer():
subprocess.Popen(["cmd", "/c", "start", "", filepath], creationflags=subprocess.CREATE_NO_WINDOW)
return
open_app = "用 "+_get_default_image_viewer_name()+" 打开"
open_action = QAction(open_app, parent_widget)
open_action.setEnabled(bool(filepath))
open_action.triggered.connect(_default_image_viewer)
menu.addAction(open_action)

  1. 跨平台 + 逻辑双重 bug:_copy_selected_photos 用 cmd copy 命令(macOS 无)、硬编码反斜杠路径分隔符、CREATE_NO_WINDOW;并且 img_path[:-4] + ".*" 假定扩展名固定 3 字符,对 .jpeg / .heic / .heif 会生成 IMG_001..* 这种错误通配符,文件根本不会被复制;Popen 又是 fire-and-forget,success_count 在文件还没复制完时就被加 1。文件已经 import shutil 但未使用——应改用 shutil.copy2

for item in selected_list:
try:
# 从字典里拿真实路径
img_path = item["current_path"]
if os.path.exists(img_path):
img_path = img_path[:-4] + ".*"
# 复制通配符文件
subprocess.Popen(f'copy "{img_path}" "{target_dir}\\"', shell=True,
creationflags=subprocess.CREATE_NO_WINDOW)
success_count += 1
except Exception as e:
# 出错时安全记录
fname = item.get("filename", "未知文件")
fail_files.append(f"{fname} -> {str(e)}")
# 4. 结果提示
msg = f"执行导出:{success_count} 张,请稍后查看"
if fail_files:
msg += f"\n失败:{len(fail_files)} 张"
QMessageBox.information(self, "完成", msg)

  1. 选择计数器回归:_on_multi_selection_changedif n > 1: 改成 if n >= 1:,与 thumbnail_grid.py 中"普通点击也加入 _multi_selected"的改动叠加后,每次单击都会显示"已选 1 张"。同文件下另一个类的同名方法 (L2068) 仍然是 n > 1,两个 ResultsBrowser 类行为不一致

@Slot(list)
def _on_multi_selection_changed(self, photos: list):
"""C3:多选状态变化,更新工具栏显示。"""
n = len(photos)
if n >= 1:
self._select_count_label.setText(self.i18n.t("browser.selected_count").format(n=n))
self._select_count_label.show()
else:
self._select_count_label.hide()
# C5:仅当选中 2 张时显示对比按钮
self._compare_btn.setVisible(n == 2)

  1. update_count 警告色阈值从 count < 10 改成 count < 1000,几乎所有正常结果集都会被画成警告色,警告提示形同虚设;PR 描述未说明此变更,疑似调试残留

warning_color = COLORS.get('warning', '#E8C000')
if count == 0:
self._count_label.setStyleSheet(
f"color: {warning_color}; font-size: 11px; background: transparent;"
)
self._count_label.setText("⚠ 无结果")
elif count < 1000:
self._count_label.setStyleSheet(
f"color: {warning_color}; font-size: 11px; background: transparent;"
)
self._count_label.setText(f"{count} 张匹配")
else:
self._count_label.setStyleSheet(
f"color: {COLORS['text_muted']}; font-size: 11px; background: transparent;"
)

  1. 启动自动更新检查被注释掉,但上方注释仍写着 # V4.0.1: 启动时检查更新(延迟2秒...),注释与代码不一致;PR 描述也未说明禁用此功能,疑似本地调试残留

# V4.0.1: 启动时检查更新(延迟2秒,避免阻塞UI,没有更新时不弹窗)
#QTimer.singleShot(2000, lambda: self._check_for_updates(silent=True))
# V4.2: 启动时预加载所有模型(延迟3秒,后台加载不阻塞UI)
QTimer.singleShot(3000, self._preload_all_models)

  1. 循环导入风险:filter_panel.py 通过 from ui.results_browser_window import set_btn_style 取用 set_btn_style,但该函数实际定义在 ui/styles.py;而 results_browser_window 又导入了 FilterPanel,仅靠局部 import 才避免循环。应直接 from ui.styles import set_btn_style

# --- 重置按钮 ---
reset_btn = QPushButton(self.i18n.t("browser.reset_filter"))
reset_btn.setObjectName("secondary")
###added by old skywalker
from ui.results_browser_window import set_btn_style
set_btn_style(reset_btn)
###end
reset_btn.clicked.connect(self.reset_all)
layout.addWidget(reset_btn)

不建议直接合并:第 1-3 条属于 macOS 阻塞性问题,违反 CLAUDE.md 的强制跨平台规则;第 4-6 条疑似调试残留或半成品,需要作者确认意图;第 7 条建议顺手修复。

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@jamesphotography
Copy link
Copy Markdown
Owner

你好,感谢你的贡献!

由于此 PR 基于较早版本,与当前 v4.2.5 代码差异较大,并且存在若干会导致 macOS 端无法启动 results browser 的跨平台问题(详见上方 Code review)。建议你以最新的 v4.2.5 分支为基础,重新整理改动后再次提交 PR,我们会优先合入。重点请关注:

  • 涉及 winreg / cmd /c start / CREATE_NO_WINDOW 等 Windows 专属代码,必须用 if sys.platform == "win32": / elif sys.platform == "darwin": 分支保护,参考 _move_to_trash 的写法。
  • 文件复制请改用 shutil.copy2(PR 已经 import shutil 但未使用),不要走 shell copy
  • 自动更新检查的 QTimer.singleShot(...) 不要注释掉。
  • update_count 警告阈值的 < 1000 看起来是调试值,请改回 < 10

非常欢迎你重新 rebase 到 v4.2.5 后再 PR,谢谢!


Hi, thanks for the contribution!

This PR is based on an older revision and has diverged significantly from the current v4.2.5 branch. It also contains several cross-platform issues that will break the results browser on macOS (see the Code review above). Could you please rebase your work on top of v4.2.5 and open a fresh PR? We'll prioritize reviewing it. Key things to address:

  • Any Windows-only code (winreg, cmd /c start, subprocess.CREATE_NO_WINDOW, etc.) must be guarded with if sys.platform == "win32": / elif sys.platform == "darwin": branches — see how _move_to_trash is structured.
  • For file copy, please use shutil.copy2 (the PR already imports shutil but never uses it) instead of shelling out to copy.
  • Don't comment out the startup auto-update check (QTimer.singleShot(2000, lambda: self._check_for_updates(...))).
  • The update_count threshold < 1000 looks like a debug value — please restore it to < 10.

Looking forward to your updated PR. Thanks again!

@jamesphotography
Copy link
Copy Markdown
Owner

你好 @Huang970,更新一下基线建议——

这边在过去几天已经把 nightly 推进到 v4.2.6-RC5(包含 PR #87 upstream migration 与多项 mac/win 构建链路修复),相对前面我建议你 rebase 的 v4.2.5 已经又往前走了不少。请直接基于 最新的 nightly / v4.2.6-RC5 重新整理你的改动并提交一个新 PR,不需要再以 v4.2.5 为基准

最新 RC5 发布页:https://github.com/jamesphotography/SuperPicky/releases/tag/v4.2.6-RC5

之前 Code Review 中提到的几个阻塞问题仍需在新 PR 中解决:

  • import winreg / cmd /c start / subprocess.CREATE_NO_WINDOW 必须用 if sys.platform == "win32": / elif sys.platform == "darwin": 分支保护(可参考 v4.2.6 中 _move_to_trashbuild_release_mac.py 的写法)
  • 文件复制改用 shutil.copy2,不要走 shell copy
  • 不要注释掉自动更新检查的 QTimer.singleShot(...)
  • update_count 警告阈值 < 1000 看起来是调试值,请改回 < 10

这个 PR 我们先关闭,等你基于 v4.2.6 重开新 PR。感谢贡献!


Hi @Huang970, updating the base-branch guidance —

Over the past few days the nightly branch has moved forward to v4.2.6-RC5 (it now includes PR #87's upstream migration plus several macOS / Windows build-pipeline fixes), so it's already past the v4.2.5 base I previously suggested. Please rebase your work on top of the current nightly / v4.2.6-RC5 and open a fresh PR — you no longer need to target v4.2.5.

Latest RC5 release: https://github.com/jamesphotography/SuperPicky/releases/tag/v4.2.6-RC5

The blocking issues from the earlier code review still need to be addressed in the new PR:

  • Windows-only code (import winreg, cmd /c start, subprocess.CREATE_NO_WINDOW) must be guarded with if sys.platform == "win32": / elif sys.platform == "darwin": (see _move_to_trash and build_release_mac.py in v4.2.6 for examples).
  • Use shutil.copy2 for file copy instead of shelling out to copy.
  • Don't comment out the startup auto-update check QTimer.singleShot(...).
  • The update_count threshold < 1000 looks like a debug value — please restore it to < 10.

Closing this PR — please reopen as a fresh PR rebased on v4.2.6 when ready. Thanks again for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants