Skip to content

Conversation

uiryuu
Copy link
Member

@uiryuu uiryuu commented Mar 25, 2023


Description:
This commit:

  • Make the leading space of the icon to 0, so that the icon can align with the search icon
  • Side bar width 220 -> 200, saves space for the content view

Before:
image

After:
Screenshot 2023-03-25 at 22 33 40

- Make the leading space of the icon to 0, so that the icon can align
  with the search icon
- Side bar width 220 -> 200
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 haven't upgraded my MacBook Air from Catalina as I am still waiting for Apple to fix some macOS regressions. Without adding new ones. Still waiting. This gives me the ability to test IINA under macOS 10.15.7.

Apple made a massive amount of incompatible stylistic changes in Big Sur. The Free Pascal Wiki page macOS Big Sur changes for developers details the changes. I've been working with @svobs and checking to make sure UI PRs look good in both Ventura and Catalina. This PR looks nice in Ventura, but this is what I see in Catalina:
pr-4292-catalina

@uiryuu
Copy link
Member Author

uiryuu commented Mar 26, 2023

Can you attach a picture of what our preference looked like on Catalina on the dev branch?

@uiryuu uiryuu marked this pull request as draft March 26, 2023 01:02
@low-batt
Copy link
Contributor

You should be ignoring low-batt and resting.

I build the develop branch and took it to my MacBook Air. This is what preferences looks like currently under Catalina:

pr-4292-develop

@uiryuu uiryuu marked this pull request as ready for review March 26, 2023 06:55
@uiryuu
Copy link
Member Author

uiryuu commented Mar 26, 2023

I revert back the changes for 10.15 and above, please check how it looks on Catalina

@low-batt
Copy link
Contributor

Worked under macOS Ventura, crashed under macOS Catalina.

With the latest commit I'm getting this warning from Xcode:

Users/low-batt/Documents/builds/pr-tests/iina/iina/Base.lproj/PreferenceWindowController.xib Outlet 'imageViewLeadingConstraint' of 'File's Owner' is connected to 'Image View.leading = leading,' an invalid destination (Object may be repeated at runtime.)

I'm guessing it is crashing on this line in PreferenceWindowController:

if #unavailable(macOS 11.0) {
…
  imageViewLeadingConstraint.constant = 20

This is the relevant portion of the crash report:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libswiftCore.dylib            	0x00007fff6eb8f1e1 closure #1 in closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) + 529
1   libswiftCore.dylib            	0x00007fff6eb8ecf7 closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) + 87
2   libswiftCore.dylib            	0x00007fff6eb8e9c3 closure #1 in _assertionFailure(_:_:file:line:flags:) + 99
3   libswiftCore.dylib            	0x00007fff6eb8e5d5 _assertionFailure(_:_:file:line:flags:) + 533
4   com.colliderli.iina           	0x0000000109a23c78 PreferenceWindowController.windowDidLoad() + 2840
5   com.colliderli.iina           	0x0000000109a24ddc @objc PreferenceWindowController.windowDidLoad() + 28
6   com.apple.AppKit              	0x00007fff32d1a42e -[NSWindowController _windowDidLoad] + 624
7   com.apple.AppKit              	0x00007fff32d161e4 -[NSWindowController window] + 110
8   com.apple.AppKit              	0x00007fff32d97fa2 -[NSWindowController showWindow:] + 36
9   com.colliderli.iina           	0x000000010993df13 AppDelegate.showPreferences(_:) + 99
10  com.colliderli.iina           	0x000000010993df64 @objc AppDelegate.showPreferences(_:) + 52

@svobs
Copy link
Contributor

svobs commented Mar 27, 2023

@uiryuu you are getting a null value for imageViewLeadingConstraint (try changing if #unavailable(macOS 11.0) to if true).

This is a constraint which you are attaching to a single NSTableCellView, inside a row of the table. In InterfaceBuilder the NSTableCellView looks like the only child element of the NSTableColumn, but that's not accurate. You should think of it as a "prototype" of a row in the table; it will be duplicated for each row. But it's not an actual instance (yet).

SCR-20230327-dmir-2

It doesn't really make sense to try to inject the constraint into PreferenceWindowController via @IBOutlet, because there's no 1-to-1 mapping for it into the window instance. What will happen is that a new instance of the constraint will be created as needed by the NSTableView, along with the rest of the each row.

Unfortunately, since this table isn't using a NSTableViewDataSource, I'm not sure I know of an easy way to put custom code into these components like you're trying to do. Maybe you can create a custom view class, or a view controller for each row (neither of which I have tried, but just throwing out ideas).

@uiryuu
Copy link
Member Author

uiryuu commented Mar 28, 2023

@uiryuu you are getting a null value for imageViewLeadingConstraint (try changing if #unavailable(macOS 11.0) to if true).

This is a constraint which you are attaching to a single NSTableCellView, inside a row of the table. In InterfaceBuilder the NSTableCellView looks like the only child element of the NSTableColumn, but that's not accurate. You should think of it as a "prototype" of a row in the table; it will be duplicated for each row. But it's not an actual instance (yet).

SCR-20230327-dmir-2

It doesn't really make sense to try to inject the constraint into PreferenceWindowController via @IBOutlet, because there's no 1-to-1 mapping for it into the window instance. What will happen is that a new instance of the constraint will be created as needed by the NSTableView, along with the rest of the each row.

Unfortunately, since this table isn't using a NSTableViewDataSource, I'm not sure I know of an easy way to put custom code into these components like you're trying to do. Maybe you can create a custom view class, or a view controller for each row (neither of which I have tried, but just throwing out ideas).

Thanks for the great info! I also did some research a little bit, and found I might have to change the table view to view based, which I would not bother doing right now. I'll convert this to draft until I find a good way to solve the issue.

@uiryuu uiryuu marked this pull request as draft March 28, 2023 00:26
@uiryuu uiryuu closed this May 9, 2023
@uiryuu uiryuu deleted the settings-tweak branch May 9, 2023 08:26
uiryuu added a commit that referenced this pull request Jan 5, 2024
This commit:
- For macOS 10.11+, make the leading space of the icon to 0, so that the icon
can align with the search icon
- Reduce the width of sidebar from 220 to 200
uiryuu added a commit that referenced this pull request Jan 6, 2024
#4750)

* Remake of #4292

This commit:
- For macOS 10.11+, make the leading space of the icon to 0, so that the icon
can align with the search icon
- Reduce the width of sidebar from 220 to 200

* Stripped unused code & formatted code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants