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

Playback History window: modernize look & improve window functionality #4257

Merged
merged 5 commits into from Mar 30, 2023

Conversation

svobs
Copy link
Contributor

@svobs svobs commented Mar 10, 2023

Before:
Screenshot 2023-03-09 at 21 20 04
After:
Screenshot 2023-03-09 at 21 00 05


Description:

Submitting this in case there is interest.

  • Seeing as the smaller "HUD" type of window is going out of style (the "round rect" style of of the "Group By" box is very antiquated), this upgrades most of the elements to full-size.
  • The File column now expands/shrinks when the window size changes.
  • Column sizes are saved across launches.
  • Increased minimum size of Played at column so that its title is not cut off.
    • Min size expands to a larger value when grouping by Folder, to help fit the longer content.
  • Fixed all warnings and added missing constraint in the XIB.
  • Disabled the "filling up" animations in the progress bars. Doubtful anyone even noticed them, and they slowed down scrolling a lot.

@low-batt
Copy link
Contributor

Glad you are working on this.

I tested macOS Catalina and changes look good there.

If you want to improve it some more there are some other issues with this window. This screen shot shows 2 issues. Note the "|" after the Played at column. The table seems to have an additional unnamed and unused column. Also the default size of the Played at column is insufficient for some values:

pr-4257-extra

If I enlarge the window the extra column is easy to see:

pr-4257-wide

Grouping by Folder results in all Played at values being truncated:

pr-4257-folder

@svobs svobs force-pushed the pr-playback-history-facelift branch from b6dedc3 to 5722605 Compare March 12, 2023 10:40
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.

Tested under macOS Ventura 13.2.1 and macOS Catalina 10.15.7. Significant improvements to the history window. Looks good to me.

@svobs
Copy link
Contributor Author

svobs commented Mar 12, 2023

@low-batt I see you noticed my push, thanks! I didn't have time to do a writeup. But I still wanted to add a few extra notes for clarity.

I played with the sizing of the table columns quite a bit, but I was unable to come up with an ideal solution which prevents the wasted space in the right gutter. It seems to be a limitation of its (very, very old) design. You may have noticed that shrinking the window manually to close that gap and then expanding it again seems to snap the rightmost column to the right side of the window as desired. That seems to be a modern addition. But Apple didn't provide any kind of handle for it, so getting that to happen programmatically is extremely difficult.

I did find this old post but the comments were not helpful. After adding up the column sizes and the intercell spacing, comparing the sum to the width of the table still left...12.5 pts...still unaccounted for. Getting the calculation off by even a pixel would create a horizontal scroll, so I don't think it's wise to try to fudge it.

I made a separate attempt to try to get the widths of the NSTextField in the right column once it was rendered, to see if I could make a "expand to fit" feature... The queries for its various widths returned either 0 or (in one case) 4.0. So, not much luck there.

So, as noted in the commit, I added a larger min-width constraint when grouping by Folder. But when resizing the column to satisfy that min-width, I'm hesitant to expand the columns because of the above inability to reliably determine how much space is available on the right, and so am, I settled instead on shrinking the other columns to make space.

OK that's enough rambling... the other comments in the commit should be self-explanatory.

@low-batt low-batt requested a review from lhc70000 March 15, 2023 02:35
@uiryuu
Copy link
Member

uiryuu commented Mar 18, 2023

@svobs
image
Can you make these numbers monospaced?

@svobs
Copy link
Contributor Author

svobs commented Mar 21, 2023

Can you make these numbers monospaced?

@uiryuu: see new commit

@svobs svobs force-pushed the pr-playback-history-facelift branch from 5663d34 to e083d6f Compare March 21, 2023 03:59
@low-batt
Copy link
Contributor

Since we are now using a fixed sized font, should the format of dates and times in Played at column be changed to eliminate the comma and align these values (adding spaces or zeros) to make it look cleaner? That column can look a bit jumbled when Group by is set to Folder. Just a thought…

@svobs
Copy link
Contributor Author

svobs commented Mar 22, 2023

Huh. I didn't actually notice that my previous fix didn't work and the font wasn't actually fixed width. It looked like it lined up and it didn't show the actual font being used... New commit uses Menlo explicitly. Not the prettiest font, but I don't think there's a prettier built-in alternative.

…mproved layout constraints & column sizing, + remembers window size & positions + column sizes & positions
…y" selection. Disable animation of Progress column in History table (scrolling is much faster now). Disable ability to reorder History table columns due to problem restoring state.
@svobs svobs force-pushed the pr-playback-history-facelift branch from 8a060f2 to f927e14 Compare March 22, 2023 00:18
Copy link
Member

@uiryuu uiryuu left a comment

Choose a reason for hiding this comment

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

Also please consider changing the search text field to rounded style

@@ -55,6 +59,7 @@ class HistoryWindowController: NSWindowController, NSOutlineViewDelegate, NSOutl

override func windowDidLoad() {
super.windowDidLoad()
self.windowFrameAutosaveName = "PlaybackHistoryWindow"
Copy link
Member

Choose a reason for hiding this comment

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

Please set the autosave name in the xib editor as we did before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also please consider changing the search text field to rounded style

So, search boxes are rounded, but all other text boxes are square?

Made new commit with requested changes.

Copy link
Member

Choose a reason for hiding this comment

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

So, search boxes are rounded, but all other text boxes are square?

But all search text fields are rounded, like the one in the key binding pref.

@uiryuu uiryuu merged commit 9b487ef into iina:develop Mar 30, 2023
2 checks passed
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

3 participants