nt4f04uNd / sweyer

Music player built with Flutter
BSD 3-Clause "New" or "Revised" License
197 stars 47 forks source link

Fix deselect all #138

Closed nt4f04uNd closed 2 months ago

nt4f04uNd commented 2 months ago

Fixes https://github.com/nt4f04uNd/sweyer/issues/98

Problem was on this line https://github.com/nt4f04uNd/sweyer/blob/fix/98/playlist-deselect-all/lib/routes/home_route/search_route.dart#L619

Also, here this code couldn't pick up the proper runtimeType

I refactored both places for double-bulletproof + other place where we were checking runtimeType, instead of contentType

Opened an issue to remove all usages of runtimeType https://github.com/nt4f04uNd/sweyer/issues/137

Abestanis commented 2 months ago

Also, should we add a test for this to avoid regressing this in the future?

Abestanis commented 2 months ago

I am trying to write a test for this, but I can't get the last pumpAndSettle to wait for the animation when deselecting everything. Maybe you have an idea what's wrong here.

group('common selection actions on all tabs', () {
    final album0 = albumWith(id: 0);
    final album1 = albumWith(id: 1);
    final album2 = albumWith(id: 2);
    final artist0 = artistWith(id: 0);
    final artist1 = artistWith(id: 1);
    final artist2 = artistWith(id: 2);
    final song0 = songWith(id: 0, title: 'Song 0', albumId: album0.id, artistId: artist0.id);
    final song1 = songWith(id: 1, title: 'Song 1', albumId: album1.id, artistId: artist1.id);
    final song2 = songWith(id: 2, title: 'Song 2', albumId: album2.id, artistId: artist2.id);
    final playlist0 = playlistWith(id: 0, songIds: [song0.id]);
    final playlist1 = playlistWith(id: 1, songIds: [song1.id]);
    final playlist2 = playlistWith(id: 2, songIds: [song2.id]);

    for (var (tabName, contentType) in [
      ('tracks', ContentType.song),
      ('album', ContentType.album),
      ('playlists', ContentType.playlist),
      ('artists', ContentType.artist),
    ]) {
      testWidgets('allows selecting all and deselecting all on the $tabName tab', (WidgetTester tester) async {
        await setUpAppTest(() {
          FakeSweyerPluginPlatform.instance.songs = [song0, song1, song2];
          FakeSweyerPluginPlatform.instance.albums = [album0, album1, album2];
          FakeSweyerPluginPlatform.instance.playlists = [playlist0, playlist1, playlist2];
          FakeSweyerPluginPlatform.instance.artists = [artist0, artist1, artist2];
        });
        await tester.runAppTest(() async {
          // Select tab
          if (contentType != ContentType.song) {
            await tester.tap(find.byIcon(contentType.icon).first);
            await tester.pumpAndSettle();
          }

          // Select first content
          await tester.longPress(find.byType(ContentTile).first);
          await tester.pumpAndSettle();

          var selectionController = ContentControl.instance.selectionNotifier.value!;
          expect(selectionController.inSelection, true);
          expect(selectionController.data.length, 1);
          expect(find.descendant(of: find.byType(SelectionCounter), matching: find.text('1')).hitTestable(),
              findsOneWidget);

          // Select all
          await tester.tap(find.byType(SelectAllSelectionAction).last);
          await tester.pumpAndSettle();

          int numElements = ContentControl.instance.getContent(contentType).length;
          expect(selectionController.inSelection, true);
          expect(selectionController.data.length, numElements);
          expect(find.descendant(of: find.byType(SelectionCounter), matching: find.text('$numElements')).hitTestable(),
              findsOneWidget);

          // Deselect all
          await tester.tap(find.byType(SelectAllSelectionAction).last);
          await tester.pumpAndSettle();

          expect(selectionController.inSelection, false);
          expect(selectionController.data.length, 0);
          expect(selectionController.animationController.isCompleted, true); // This fails
          expect(selectionController.animation.isCompleted, true); // This fails

          expect(find.byType(SelectAllSelectionAction), findsNothing); // This fails
          expect(find.byType(SelectionCounter), findsNothing); // This fails
        });
      });
    }
  });
nt4f04uNd commented 2 months ago

A have created #142 that adds the tests for this, so we can merge this. 👍

Thanks!