-
Notifications
You must be signed in to change notification settings - Fork 394
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
Refactor Setting - Service and provide service configuration view #326
Conversation
#324 |
I tried this change, and it looks good! Meanwhile, I've noticed some minor UI issues:
|
The rewritten |
For point 4, I don't this text style should be secondary. Usually, secondary mean disabled. It may confuse user. @tisfeng How do you think? |
For point 4 I think you are right. Let's keep the current setting. |
Currently, it looks good to me, do you have any good idea for this? @phlpsong |
No other issues, LGTM |
ok. |
I'll add the rest of the service configuration later in the evening when I have time. |
This feature is almost completed, it's recommended to approve & merge before start add other configurations. |
I tested this code with no problems. The only issue is that switching tab multiple times may cause lag, but this seems to be an internal issue with the |
@CanglongCl This is a strange issue, I have tested it and found that when the program is first running, switching Tabs doesn't cause the problem, it seems that after running for a while, switching Tabs causes service-issue.mp4 |
I didn't reproduce the issue. But I did a small refactor so now only the windowType changes will trigger calling to |
Fixed |
I tested the new code, it looks good to me 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Please review the code, if no problem, I will merge this PR tonight. |
I tested the latest commit, looks good to me 🎉 |
For point 1 & 4, I think it SwiftUI's issue. For point 2, I think it is the default behavior of For point 4, I think we can find a way to fix it, but I'm not familiar to window management in SwiftUI. Maybe we should handle it in another PR. |
For point 4 I think it's a SwiftUI issue, same problem here https://developer.apple.com/forums/thread/731628. |
@Jerry23011 You're very observant. 👍 For points 1 and 3, there does seem to be a slight issue, but it's not very noticeable perceptually, probably because there are more elements in the General Tab, not sure if it's a performance issue with SwiftUI. For points 2 and 4, release mode performs much better compared to debug mode, seems strange. For this kind of problem, which is not very serious, we can check it a little bit, and if it's a problem with SwiftUI itself, we can ignore it for now and leave it to Apple to fix it. |
@Jerry23011 You can open a new issue to record these problem, we can handle them in another PR. If there are no more questions, I will merge this PR later. |
okok |
No description provided.