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

Remove code for supporting older systems #4977

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

uiryuu
Copy link
Member

@uiryuu uiryuu commented Jun 5, 2024

Description:
Remove code previously used for 10.11 - 10.14 systems. The major ones include a lot of colors, appearance, PIP and touchbar support.

Copy link
Contributor

@low-batt low-batt left a comment

Choose a reason for hiding this comment

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

I reviewed all of the code changes. I pulled the PR, built IINA and tested under macOS Ventura and macOS Catalina.

Changes look good to me.

We should hold off doing more refactoring until we have merged more of the outstanding PRs to avoid merge conflicts.

@@ -1127,11 +1125,11 @@
<point key="canvasLocation" x="14" y="-292"/>
</customView>
<customView id="1fz-oP-RhZ" userLabel="Prefs &gt; UI &gt; PIP">
<rect key="frame" x="0.0" y="0.0" width="480" height="124"/>
<rect key="frame" x="0.0" y="-14" width="480" height="138"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess Xcode makes odd changes like setting y to negative values due to constraints not being quite right? We have quite a few problems in the XIBs that should be fixed at some point.

mainView.layer?.backgroundColor = CGColor(gray: 0.1, alpha: 1)
visualEffectView.material = .ultraDark
window.appearance = NSAppearance(iinaTheme: theme)
if #available(macOS 10.16, *) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I recall something about needing to retain #available referencing 10.16 instead of 11 due to the need to support old SDKs. We might want to consider cutting off support for older Xcode versions, updating this, and allow use of the latest Swift features. But first we should merge a lot of the outstanding PRs before doing refactoring that can introduce merge conflicts.

@low-batt
Copy link
Contributor

low-batt commented Jun 7, 2024

I may need to pull my approval. Hang on…

@low-batt
Copy link
Contributor

low-batt commented Jun 7, 2024

I confirmed what I found is a regresison in develop and not due to this PR.

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.

None yet

2 participants