Refactor settings menu in swiftui only UI

[liyafly]

#286 初步完成UI，功能是否需要等使用swift重构后添加还是使用桥接Objc完成呢？

[tisfeng]

一些简单的功能，可以在重构 UI 时顺便也重写了。一些比较复杂的，像输入翻译这几个，可以先桥接使用 objc，后面专门弄个 PR 来重写。

[tisfeng]

目前这个【检查更新】功能好像不对，麻烦你也看一下。

[liyafly]

我已经添加objc桥接，但debug下部分功能系统权限授权有问题，无法做到全面测试。请review

[tisfeng]

有两个小问题：

• @Property window 出现警告，可能是桥接文件添加 #import "EZWindowManager.h" 导致。
• Sparkle 有 gentle reminders 警告，我之前是通过实现 supportsGentleScheduledUpdateReminders 消除了这个警告。

[tisfeng]

我已经添加objc桥接，但debug下部分功能系统权限授权有问题，无法做到全面测试。请review

参考下面，你可以在 Easydict-debug.xcconfig 配置文件添加你自己的开发者账号，方便后续调试。

DEVELOPMENT_TEAM = 79NQA2XYHM
CODE_SIGN_IDENTITY = Apple Development
CODE_SIGN_STYLE = Manual

由于这个文件不应该提交，可以使用 git 忽略这个文件变动。（后面假如切换分支遇到问题，可以再改回来）

git update-index --skip-worktree Easydict-debug.xcconfig

[liyafly]

有两个小问题：

• @Property window 出现警告，可能是桥接文件添加 #import "EZWindowManager.h" 导致。
• Sparkle 有 gentle reminders 警告，我之前是通过实现 supportsGentleScheduledUpdateReminders 消除了这个警告。

好的，我下周进行修改。thanks

[liyafly]

有两个小问题：

• @Property window 出现警告，可能是桥接文件添加 #import "EZWindowManager.h" 导致。

• Sparkle 有 gentle reminders 警告，我之前是通过实现 supportsGentleScheduledUpdateReminders 消除了这个警告。

I added the nullable modifier to @property (nullable, nonatomic, weak) EZBaseQueryWindow *window;, but Xcode is still throwing a warning. I’m not quite sure why this is happening. Do you have any insights on this issue?

[tisfeng]

Wait for me to take a look.

[tisfeng]

It's strange, I've tried a number of ways and none of them work. This is caused by being referenced by a Swift bridge file, and I'm not familiar with Swift.

If there's really no way to fix it, we can just ignore the warning:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wnullability"
@property (nonatomic, weak) EZBaseQueryWindow *window;
#pragma clang diagnostic pop

Ref: https://stackoverflow.com/a/39918786/8378840

[Kyle-Ye]

It's strange, I've tried a number of ways and none of them work. This is caused by being referenced by a Swift bridge file, and I'm not familiar with Swift.

If there's really no way to fix it, we can just ignore the warning:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wnullability"
@property (nonatomic, weak) EZBaseQueryWindow *window;
#pragma clang diagnostic pop

Ref: stackoverflow.com/a/39918786/8378840

Nope. This is just a small issue and we should fix it easily.

The log message already tells us how to fix it.

/Users/kyle/Github/Easydict/Easydict/Feature/ViewController/Window/BaseQueryWindow/EZBaseQueryViewController.h:21:48: warning: conflicting nullability specifier on return types, '_Nullable' conflicts with existing specifier 'nonnull' [-Wnullability]
@property (nonatomic, weak) EZBaseQueryWindow *window;
                                               ^
In file included from /Users/kyle/Github/Easydict/Easydict/Feature/PerferenceWindow/EZSettingViewController.m:12:
/Users/kyle/Github/Easydict/Easydict/Feature/Utility/EZCategory/NSViewController+EZWindow.h:15:1: note: previous declaration is here
- (NSWindow *)window;
^
1 warning generated.

weak just infer nullable implicitly. I believe the right fix is to remove NS_ASSUME_NONNULL_BEGIN and NS_ASSUME_NONNULL_END in NSViewController+EZWindow.h or add a nullable mark here(Suggested).

NS_ASSUME_NONNULL_BEGIN

@interface NSViewController (EZWindow)

- (NSWindow *)window;

@end

NS_ASSUME_NONNULL_END

NS_ASSUME_NONNULL_BEGIN

@interface NSViewController (EZWindow)

-- (NSWindow *)window;
+- (nullable NSWindow *)window;
@end

NS_ASSUME_NONNULL_END

You can just merge the patch here liyafly#1.

[tisfeng]

Awesome, I thought it was warning me about @property (nonatomic, weak) EZBaseQueryWindow *window;, didn't look at the log details carefully 😥

[liyafly]

It's strange, I've tried a number of ways and none of them work. This is caused by being referenced by a Swift bridge file, and I'm not familiar with Swift.
If there's really no way to fix it, we can just ignore the warning:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wnullability"
@property (nonatomic, weak) EZBaseQueryWindow *window;
#pragma clang diagnostic pop

Ref: stackoverflow.com/a/39918786/8378840

Nope. This is just a small issue and we should fix it easily.

The log message already tells us how to fix it.

/Users/kyle/Github/Easydict/Easydict/Feature/ViewController/Window/BaseQueryWindow/EZBaseQueryViewController.h:21:48: warning: conflicting nullability specifier on return types, '_Nullable' conflicts with existing specifier 'nonnull' [-Wnullability]
@property (nonatomic, weak) EZBaseQueryWindow *window;
                                               ^
In file included from /Users/kyle/Github/Easydict/Easydict/Feature/PerferenceWindow/EZSettingViewController.m:12:
/Users/kyle/Github/Easydict/Easydict/Feature/Utility/EZCategory/NSViewController+EZWindow.h:15:1: note: previous declaration is here
- (NSWindow *)window;
^
1 warning generated.

weak just infer nullable implicitly. I believe the right fix is to remove NS_ASSUME_NONNULL_BEGIN and NS_ASSUME_NONNULL_END in NSViewController+EZWindow.h or add a nullable mark here(Suggested).

NS_ASSUME_NONNULL_BEGIN

@interface NSViewController (EZWindow)

- (NSWindow *)window;

@end

NS_ASSUME_NONNULL_END

NS_ASSUME_NONNULL_BEGIN

@interface NSViewController (EZWindow)

-- (NSWindow *)window;
+- (nullable NSWindow *)window;
@end

NS_ASSUME_NONNULL_END

You can just merge the patch here liyafly#1.

Okay, I didn't see the whole log either, but according to you, does this duplicate the attribute definition in the category @tisfeng

[tisfeng]

No, they are not the same. I didn't notice that they had the same name before. Maybe we can rename the name of window to baseQueryWindow, like this

@property (nonatomic, weak) EZBaseQueryWindow *baseQueryWindow;

[liyafly]

No, they are not the same. I didn't notice that they had the same name before. Maybe we can rename the name of window to baseQueryWindow, like this

@property (nonatomic, weak) EZBaseQueryWindow *baseQueryWindow;

Certainly, I’ll go through everything to ensure that all the renaming is correct

[tisfeng]

I tried renaming the window to baseQueryWindow and it looks like the warning is resolved?

[liyafly]

Yes, It would be nice to rename it, because weak just infer nullable, and it would be clearer to add nullable, don't you think? @tisfeng

[tisfeng]

Yes, It would be nice to rename it, because weak just infer nullable, and it would be clearer to add nullable, don't you think? @tisfeng

Yes, I just want to confirm that renaming the property can fix this warning, and also explicitly adding nullable is ok.

[tisfeng]

I ran the code, and there was no problem.

If there are no other questions, we can continue to review.

[liyafly]

I ran the code, and there was no problem.

If there are no other questions, we can continue to review.

I'm not too familiar with SwiftUI, so feel free to point out any code issues — I'm here to learn. I've tested the features extensively and they seem fine for now. Welcome to review and give feedback!

My English isn't very good, please bear with me.