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

Feat: SourceFolders #1592

Merged
merged 32 commits into from
Aug 27, 2024
Merged

Conversation

amirsaam
Copy link
Member

Originally #880, messed up the codes so I needed to detach my last commit to resolve conflicts again.

@amirsaam amirsaam marked this pull request as draft July 16, 2024 10:41
@amirsaam amirsaam requested a review from Depal1 July 16, 2024 12:08
@amirsaam amirsaam marked this pull request as ready for review July 16, 2024 12:08
@amirsaam amirsaam added enhancement New feature or request UI User interface related changes squash Indicates whether a PR must be squashed before being merged labels Jul 16, 2024
@amirsaam amirsaam changed the title SourceFolders (Detached Commit of #880) SourceFolders (Detached from #880) Jul 16, 2024
StoreVM SourceResolve Improvement
Switch to NavigationStack package (not mine, should be updated after my PR gets merged)
@TheMoonThatRises
Copy link
Member

Can this be implemented without the Semaphore and NavigationStack-backport packages? It would be nice to not include too many external packages, as most of this can be done in pure Swift (as for NavigationStack, I was thinking it could be a feature restricted to the earliest MacOS version that supports it). @Depal1?

@amirsaam
Copy link
Member Author

amirsaam commented Aug 6, 2024

@TheMoonThatRises

NavigationStack is available in macOS 13+ so if we go native, we need to bump the lowest supported OS that was not an option, so I created my own version that for some reason you and depal had issues in the tests that I didn't so I switched to this Package, to make it available to macOS 12 too. As this feature, changes Sources JSON structure it will add tons of codes in if available conditions if we add it as an version dependent feature both in UI and logic.

For Semaphore, it blocks asynchronous tasks w/o freezing UI, like we have a resolveSource function that calls an async loading api function, so if we don't block the UI from updating w/o freezing it it will make duplicates of sources in the UI (not in logic), be my guest to suggest a workaround but personally I don't invent wheels.

@Depal1
Copy link
Member

Depal1 commented Aug 11, 2024

Can this be implemented without the Semaphore and NavigationStack-backport packages? It would be nice to not include too many external packages, as most of this can be done in pure Swift (as for NavigationStack, I was thinking it could be a feature restricted to the earliest MacOS version that supports it). @Depal1?

PlayCover runs fairly good in macOS 12, as a matter of fact it might be the best macOS version to run it (due to the obnoxious changes to Mac Catalyst since Ventura), so I am a bit adamant of cutting it out from the supported macOS version list if the reason behind the change is UI. Making NavigationStack related features macOS13+ only might be best instead of adding an additional package.

@amirsaam
Copy link
Member Author

Making NavigationStack related features macOS13+ only might be best instead of adding an additional package.

@Depal1 you know that adds tons of #if available codes, hence I do volunteer to do it and add the fancy on top of it for macOS13+ for it too.
This approach to recreate the Stack in macOS12 was why you guys in first place asked for it.
I really tried to fix the issues but the behavior in different devices as you saw is outta control, I didn't have those issues on my device while you had.

@amirsaam amirsaam closed this Aug 14, 2024
@amirsaam amirsaam reopened this Aug 14, 2024
@amirsaam amirsaam changed the title SourceFolders (Detached from #880) SourceFolders Aug 14, 2024
@amirsaam amirsaam changed the title SourceFolders Feat: SourceFolders Aug 23, 2024
Copy link
Member

@Depal1 Depal1 left a comment

Choose a reason for hiding this comment

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

The minimum width of the sidebar is not displaying the name 'IPA Library' completely since the dropdown arrow was added.

@amirsaam amirsaam requested a review from Depal1 August 24, 2024 11:25
1. Makes sure Sidebar width shows IPALibrary completely
2. Marks `appendSourceData()` as a private function
@amirsaam
Copy link
Member Author

@Depal1 fixed.

@TheMoonThatRises
Copy link
Member

There seems to be an issue where switching from a source folder to the IPA Library will not display anything. Is this a reproducible issue @Depal1?

@Depal1
Copy link
Member

Depal1 commented Aug 26, 2024

There seems to be an issue where switching from a source folder to the IPA Library will not display anything. Is this a reproducible issue @Depal1?

I am not being able to reproduce it on my end. Switching from a source folder to IPA Library is displaying the IPAs from all sources.

@Depal1 Depal1 merged commit 079bd57 into PlayCover:develop Aug 27, 2024
1 check passed
@amirsaam amirsaam deleted the detatched-sourcefolders branch August 27, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request squash Indicates whether a PR must be squashed before being merged UI User interface related changes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants