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

Refactored a ton of files. #55

Merged
merged 1 commit into from
Feb 8, 2022

Conversation

ErrorErrorError
Copy link
Contributor

Hello!

In this pull request I refactored a ton of logic that to me seemed a bit redundant, there are still a couple more that I would like to refactor but I will probably work on that later. I removed the old AppKit stuff, like Main.storyboard and AppDelegate.swift in favor of the SwiftUI version of lifecycle.

I also improved App state by creating an AppStateViewModel which handles any shared data between views and publishes on data change across views.

There are some couple performance improvements and some ui improvement, such as a Preference Panel and we can add a general settings tab for any additional settings in the future.

Feel free to change anything you believe there should be change! :)

Screen Shot 2022-02-06 at 2 35 27 PM

Screen Shot 2022-02-06 at 2 33 38 PM

- Moved building the status bar in `NativeYoutubeApp.swift`
- Created a Settings view so that we don't have to use `createNewWindow`.
- Cleaned up views and reorganized PreferencesView.
- Remove redundant environmentObject
- Moved `SettingsView.swift`, now known as `YoutubePreferencesView.swift`, to `Setting` Scene.
- Created an `AppStateViewModel.swift` to share with other views about its state so we can isolate `SettingsViewModel`, which is now `YoutubePreferenceViewModel`.
- Moved specific view states from ObservableObject to the view
- Refactored `PlayListModel.swift` and `SearchModel.swift` to `VideoModel.swift`
- SearchRequest and PlaylistRequest now fetch only videos with the data it requires to play videos..
- Refactored `PlayListRowView.swift` and `SearchRowView.swift` as one since they both show only videos as of now.
- Improve showing ui when searching or loading videos
- Improve button appearance for search button
- Fixed issue when running shell command would block main thread.
@Aayush9029 Aayush9029 self-assigned this Feb 6, 2022
@Aayush9029
Copy link
Owner

Most of the stuff looks neat! Definitely liking the new changes!

@Aayush9029 Aayush9029 merged commit c87fa4f into Aayush9029:main Feb 8, 2022
@Aayush9029
Copy link
Owner

Will merge this, will be merging linted codebase as well!

@ErrorErrorError
Copy link
Contributor Author

Are there any features that you would still want to add in the future? I can possibly help out!

@Aayush9029
Copy link
Owner

Are there any features that you would still want to add in the future? I can possibly help out!

if you’re up for it feel free to explore https://github.com/Aayush9029/NativeYoutube/tree/main/NativeYoutube/AVPlayer

This will eliminate the need for IINA to be installed just to play music!
we could add VLC kit if AVPlayer isn’t enough. (networking wise)


I was also thinking of adding URL schemes to play videos on the app by opening

nativeyoutube://video-id

Which will act as a foundation for us to create safari extensions, raycast /Alfred extensions, terminal app and much more.

lmk what you think.

@ErrorErrorError
Copy link
Contributor Author

if you’re up for it feel free to explore https://github.com/Aayush9029/NativeYoutube/tree/main/NativeYoutube/AVPlayer

This will eliminate the need for IINA to be installed just to play music! we could add VLC kit if AVPlayer isn’t enough. (networking wise)

I'll take a look and see what I can do about it. Most likely we will have to use VLCKit but I will try to see if I can do it without it.

I was also thinking of adding URL schemes to play videos on the app by opening

nativeyoutube://video-id

Which will act as a foundation for us to create safari extensions, raycast /Alfred extensions, terminal app and much more.

I created a feature request for that here: #58.

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