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

[macOS] Update Info window. #2269

Merged
merged 1 commit into from Dec 12, 2021
Merged

[macOS] Update Info window. #2269

merged 1 commit into from Dec 12, 2021

Conversation

nevack
Copy link
Member

@nevack nevack commented Dec 5, 2021 •
edited

Use SF Symbols on macOS 11+

Use NSSegmentedControl instead of custom view.

Add NSImage extension to fallback loading from
assets on macOS 10.15 and older.

Also should fix sizing issues when changing pane from Options
to Files.

Fixes #2247

Result:

@GaryElshaw
Copy link
Contributor

Killing me with expectation...

nevack
nevack commented Dec 6, 2021
View reviewed changes
macosx/InfoWindowController.mm Outdated Show resolved Hide resolved
@ckerr ckerr requested a review from livings124 December 6, 2021 23:18
@ckerr ckerr added scope:mac type:refactor A code change that neither fixes a bug nor adds a feature labels Dec 6, 2021
@nevack
Copy link
Member Author

nevack commented Dec 10, 2021

Hi, @livings124. Can you take a look?

livings124
livings124 reviewed Dec 10, 2021
View reviewed changes
Copy link
Member

@livings124 livings124 left a comment

Choose a reason for hiding this comment

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

This is looking great. My last concern is just the server rack for activity, since I think the icon itself is a bit unclear what it is and BitTorrent is by design not server-based. Maybe the previous icon what fine there after all?

@nevack
Copy link
Member Author

nevack commented Dec 10, 2021

server rack for activity

It's not for activity, it's for Tracker tab

@livings124
Copy link
Member

server rack for activity

It's not for activity, it's for Tracker tab

Eep, my mistake. But I think the reinforces that it's a bit unclear. I'll give a look through the symbols, but the previous antenna might be best after all.

@nevack
Copy link
Member Author

nevack commented Dec 10, 2021

server rack for activity

It's not for activity, it's for Tracker tab

Eep, my mistake. But I think the reinforces that it's a bit unclear. I'll give a look through the symbols, but the previous antenna might be best after all.

Want to note that it's important to look symbol availability, some of them are available only on macOS 12/ iOS 15

@mangosplat
Copy link

Suggestion: perhaps chart.pie or arrow.right.arrow.left.square would make for a better representation of the 'Activity' tab (visually shows transfer progress so I think the pie chart makes sense; arrows to represent transfer)

image

image

@GaryElshaw
Copy link
Contributor

server rack for activity

It's not for activity, it's for Tracker tab

Eep, my mistake. But I think the reinforces that it's a bit unclear. I'll give a look through the symbols, but the previous antenna might be best after all.

Want to note that it's important to look symbol availability, some of them are available only on macOS 12/ iOS 15

My 2 cents is that everything looks great! My preference is for the 'Activity' icon to be a jigsaw piece rather than a hash symbol, mostly because, unless there was a chessboard icon, the information reflects parts of a whole.

@nevack
[macOS] Update Info window.
c38ee22 
Use SF Symbols on macOS 11+

Use NSSegmentedControl instead of custom view.

Add NSImage extension to fallback loading from
assets on macOS 10.15 and older.

Also should fix sizing issues when changing pane from Options
to Files.

Fixes transmission#2247
@nevack
Copy link
Member Author

nevack commented Dec 12, 2021

@livings124

  • Changed back Tracker tab icon.
  • Updated Activity tab icon.
  • Update FavIcon in Trackers list with SF Symbol.

Info

@livings124
Copy link
Member

Thanks- looks good!

@livings124 livings124 merged commit 2dbfec8 into transmission:master Dec 12, 2021
@GaryElshaw
Copy link
Contributor

GaryElshaw commented Dec 12, 2021 •
edited

Oops. MacOS 11.6.1 | 3.00+ (2b3fbd9) @nevack @livings124

Screenshot 2021-12-13 at 11 13 46 AM

@nevack
Copy link
Member Author

nevack commented Dec 13, 2021

Oops. MacOS 11.6.1 | 3.00+ (2b3fbd9) @nevack @livings124

Screenshot 2021-12-13 at 11 13 46 AM

My bad, I've made the mistake that I tried to prevent others from.

image

I'll make fix ASAP

nevack added a commit to nevack/transmission that referenced this pull request Dec 13, 2021
@nevack
[macOS] Use old icon name for Square Grid to support older macOS
e73395e 
`square.grid.3x3.fill.square` is deprecated name for `square.grid.3x3.square`.
The former is available on macOS 11.0+, while the latter is for macOS 12.0+.
They are the same visually.

Fixes this report transmission#2269 (comment)
@nevack nevack mentioned this pull request Dec 13, 2021
Merged
@nevack nevack deleted the nevack/macos-info-window branch December 13, 2021 16:12
@mikedld
Copy link
Member

mikedld commented Dec 26, 2021

@nevack, could you please look into this, sounds related: https://forum.transmissionbt.com/viewtopic.php?t=20925&p=80299

@nevack
Copy link
Member Author

nevack commented Dec 26, 2021

@nevack, could you please look into this, sounds related: https://forum.transmissionbt.com/viewtopic.php?t=20925&p=80299

Hmm, maybe some bug on older macOS versions.
Need more info about environment, then I can debug it in VM

@livings124
Copy link
Member

livings124 commented Dec 28, 2021 •
edited

@nevack, could you please look into this, sounds related: https://forum.transmissionbt.com/viewtopic.php?t=20925&p=80299

Hmm, maybe some bug on older macOS versions. Need more info about environment, then I can debug it in VM

@nevack Sounds like it's macOS 10.14.6

@mangosplat mangosplat mentioned this pull request Feb 12, 2022
Closed
@beren12
Copy link

beren12 commented Feb 17, 2022

Yes, it is a bug on older os x, what info do you need? it happens on 10.13.

@livings124
Copy link
Member

Yes, it is a bug on older os x, what info do you need? it happens on 10.13.

If you open up Console.app, when you try to change sections does it print anything?

@beren12
Copy link

beren12 commented Feb 20, 2022 via email

@livings124
Copy link
Member

@beren12 I merged in #2665 which should remove those warnings. Could you check to see if the issue remains?

@beren12
Copy link

beren12 commented Feb 21, 2022 •
edited

The warning is gone, the info pane doesn't change. Nothing in the log when I try to change it.

@beren12
Copy link

beren12 commented Mar 18, 2022

still the same as of https://build.transmissionbt.com/job/trunk-mac/7738/

@RobFog
Copy link
Contributor

RobFog commented Apr 2, 2022

There aren't many issues left on the Transmission 4 beta milestone. #2599 is one of them. @nevack, are you still interested in fixing this?

@nevack
Copy link
Member Author

nevack commented Apr 3, 2022

There aren't many issues left on the Transmission 4 beta milestone. #2599 is one of them. @nevack, are you still interested in fixing this?

Hi, I have set up VM with 10.14.6 and reproduced the issue.
Quite interesting, [InfoWindowController setTab:] is called, while [NSControl selectedTag] always returns zero.

I'll make PR as soon as solution is found

@sweetppro sweetppro mentioned this pull request Apr 3, 2022
Closed
@sweetppro
Copy link
Collaborator

I actually was also just looking at this, and submitted a fix:
#2869

@nevack
Copy link
Member Author

nevack commented Apr 3, 2022

I actually was also just looking at this, and submitted a fix: #2869

Damn, #2870 😄

@sweetppro
Copy link
Collaborator

I actually was also just looking at this, and submitted a fix: #2869

Damn, #2870 😄

ha nice!
Ill remove mine.
your PR fits the coding style better I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@livings124 livings124 livings124 left review comments

Assignees
No one assigned
Labels
scope:mac type:refactor A code change that neither fixes a bug nor adds a feature
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[macOS] Update Inspector dialog with SFSymbols on 11.0+
9 participants
@nevack @GaryElshaw @livings124 @mangosplat @mikedld @beren12 @RobFog @sweetppro @ckerr