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

Jump to bottom

feat: Support setting the horizontal and vertical screen orientation … #1258

Merged
merged 1 commit into from
Nov 28, 2023
Merged

feat: Support setting the horizontal and vertical screen orientation … #1258

merged 1 commit into from
Nov 28, 2023

Conversation

AntonioShare
Copy link
Contributor

Support setting the horizontal and vertical screen orientation in the plugin's AndroidManifest. And set the TestActivityOrientation direction in AndroidManifest to landscape

@tencent-adm
Copy link
Member

tencent-adm commented Nov 27, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@shifujun shifujun self-requested a review November 27, 2023 09:57
@shifujun
Copy link
Collaborator

感谢贡献代码。这个PR看起来很干净，应该很好合入。目前CI检查已经有点报错。你可以自己修改提交，或者我可以晚些帮你改好合入。

@shifujun
Copy link
Collaborator

CI会检查每个提交，而不仅仅是最终状态。所以你需要rebase提交，然后force push。

@AntonioShare
Copy link
Contributor Author

CI会检查每个提交，而不仅仅是最终状态。所以你需要rebase提交，然后force push。

现在应该可以了

@shifujun
Copy link
Collaborator

CI会检查每个提交，而不仅仅是最终状态。所以你需要rebase提交，然后force push。

现在应该可以了

你看这个PR还是包含两个提交，这应该是不对的。你应该把第二个fix提交合并到最初的提交中。同时修改最初的提交的commit message，使其通过检查。code format检查确实是只检查最终代码，但是commit message检查是要检查每一个提交的。

@AntonioShare
Copy link
Contributor Author

CI会检查每个提交，而不仅仅是最终状态。所以你需要rebase提交，然后force push。

非常抱歉，似乎还有问题，我将再次检查一下代码在进行force push

@AntonioShare
Copy link
Contributor Author

CI会检查每个提交，而不仅仅是最终状态。所以你需要rebase提交，然后force push。

现在应该可以了

你看这个PR还是包含两个提交，这应该是不对的。你应该把第二个fix提交合并到最初的提交中。同时修改最初的提交的commit message，使其通过检查。code format检查确实是只检查最终代码，但是commit message检查是要检查每一个提交的。

好的，非常抱歉。我这就更改

@shifujun
Copy link
Collaborator

另外你如果愿意自己动手的话，也可以在自己fork的仓库开启github action。这样可以在自己的仓库触发同样的CI检查。

@AntonioShare AntonioShare force-pushed the dev_orientation branch 2 times, most recently from f36245d to 21fa9f8 Compare November 27, 2023 10:51
@AntonioShare
Copy link
Contributor Author

另外你如果愿意自己动手的话，也可以在自己fork的仓库开启github action。这样可以在自己的仓库触发同样的CI检查。

好的，最后一次我已经检验过了，再次感谢您的指点

@AntonioShare
Copy link
Contributor Author

另外你如果愿意自己动手的话，也可以在自己fork的仓库开启github action。这样可以在自己的仓库触发同样的CI检查。

非常抱歉，最后一次多了一个.字符忘记删，已修正

@AntonioShare
Copy link
Contributor Author

另外你如果愿意自己承担的话，也可以在自己fork的仓库开启github操作。这样就可以在自己的仓库触发同样的CI检查。

检测已经通过，代码似乎还需您review一下才能进去

shifujun
shifujun requested changes Nov 27, 2023
View reviewed changes
Copy link
Collaborator

@shifujun shifujun left a comment

Choose a reason for hiding this comment

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

两个问题。1是前面评论的3次取同一域变量的问题。2是commit message可以清楚的写出来是怎么support的，即判断requestedOrientation和插件manifest中不一致时主动设置它。

...re/loader/src/main/kotlin/com/tencent/shadow/core/loader/delegates/ShadowActivityDelegate.kt Outdated
Comment on lines 191 to 194
if (pluginActivityInfo.screenOrientation != ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED && pluginActivityInfo.screenOrientation != pluginActivity.requestedOrientation) {
pluginActivity.requestedOrientation = pluginActivityInfo.screenOrientation
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

pluginActivityInfo.screenOrientation重复出现了3次。应该先赋值给局部变量，再使用，表达出3处值是同一个不变值。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的，已修正，重新force push了

@AntonioShare
feat: Support screen orientation in plugin's AndroidManifest
67da8b3 
判断requestedOrientation和插件manifest中不一致时主动设置它
@shifujun shifujun merged commit a8ca3f2 into Tencent:master Nov 28, 2023
9 checks passed
@shifujun
Copy link
Collaborator

done🎉

@shifujun shifujun mentioned this pull request Dec 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shifujun shifujun shifujun approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AntonioShare @tencent-adm @shifujun