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

Improve playlist dialogs, use only one layout #3930

Closed
wants to merge 2 commits into from

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Jul 24, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Use the same class and the same layout both to append an item to a local playlist and to choose a local/remote playlist tab. The resulting PlaylistDialog class is the result of a merge between the old local.dialog.Playlist*Dialog and SelectPlaylistFragment classes. In particular:

  • The custom adapter found in SelectPlaylistFragment has been kept, to allow for both local and remote items to exist in the list
  • The correct state saving behaviour is taken from local.dialog.PlaylistDialog
    Fixes ui issues with the select playlist tab dialog, which didn't show uploaders and stream counts, looked strange and had double title on Android 4.4.

Depends on

#3982 (due to bug report fix)

Testing apk

Tested on emulated Android 4.4 and device Android 7.0.
app-debug.zip

Agreement

@Stypox Stypox added bug Issue is related to a bug GUI Issue is related to the graphical user interface labels Jul 24, 2020
@TobiGr
Copy link
Member

TobiGr commented Jul 24, 2020

I did not test if this is also in the current version, but the info should be aligned properly.
NewPipe_playlist-dialogs

@Stypox
Copy link
Member Author

Stypox commented Jul 25, 2020

@TobiGr that's unrelated from this PR, let's create an issue

@ghost
Copy link

ghost commented Jul 25, 2020

Add to playlist button make app crash :(
https://i.imgur.com/VQEVqqa.jpg
I paste an "log" yes "log" because is empty.

Exception

  • User Action: ui error
  • Request: App crash, UI failure
  • Content Country: IT
  • Content Language: it-IT
  • App Language: it_IT
  • Service: none
  • Version: 0.19.6
  • OS: Linux Android 4.4.4 - 19
Crash log



I think Log have problem

@Stypox
Copy link
Member Author

Stypox commented Jul 25, 2020

Hi @domiuns , thank you for testing ;-)
Unfortunately I am unable to reproduce on the emulator. Could you point out the exact steps leading to the crash since opening the app?

Since you have Android 4.4, would you like to join the testers (@mentioned in PRs needing tests on old devices)? If so, please add yourself here: https://github.com/TeamNewPipe/NewPipe/wiki/Testers

@ghost
Copy link

ghost commented Jul 25, 2020

Hi @Stypox the app crash happens in all sections, I just press add to the playlist (I loaded the zip file with the old configurations) but I also tried with a clean installation and the app continues to crash. try the feed section or the trend section.

I have add my "name" to the tester list, i want help this project :) i'm not a programmer but I want to help you as much as I can.

setReportSenderFactoryClasses() is deprecated, now extensions (ReportSenderFactory is an extension) should be registered using AutoService: https://github.com/ACRA/acra/wiki/Custom-Extensions#by-annotation
Use same class (by extending it) both to append an item to a playlist and to choose a playlist tab
Fixes ui issues with select playlist tab dialog
@Stypox
Copy link
Member Author

Stypox commented Jul 28, 2020

@domiuns in #3982 I fixed the problem you had with missing stack trace. Could you test this new apk and provide me the bug report, which should now be non-empty?
app-debug.zip

@litetex
Copy link
Member

litetex commented Oct 1, 2021

@Stypox
Any progress here?
Or should we close this?

@litetex litetex marked this pull request as draft October 1, 2021 17:45
@Stypox
Copy link
Member Author

Stypox commented Oct 6, 2021

This would have to be redone from scratch, as there have been changes in the meantime. Closing it

@Stypox Stypox closed this Oct 6, 2021
@litetex
Copy link
Member

litetex commented Oct 10, 2021

Some cleanup will be done with #7194

@litetex litetex mentioned this pull request Oct 10, 2021
2 tasks
@Stypox Stypox deleted the playlist-dialogs branch August 4, 2022 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug GUI Issue is related to the graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

View hierarchy touched by a wrong thread
3 participants