Use the new broadcast/top endpoint #806
Conversation
81163ad to
97c0086
Compare
|
I'll review asap, in the meantime can you re-run the script to solve the conflicts? thanks! |
e607562 to
96233d3
Compare
| BroadcastRound? get curentRound => | ||
| rounds.where((r) => r.status == RoundStatus.live).firstOrNull ?? | ||
| rounds.where((r) => r.status == RoundStatus.finished).lastOrNull; | ||
| factory Broadcast.loading() => Broadcast( |
There was a problem hiding this comment.
Instead of that Broadcast.loading() that has empty name and id, is it possible to have a nullable broadcast field where the broadcast is not yet present?
I believe it is only used in BroadcastPicture for now, so it should be possible.
There was a problem hiding this comment.
Where should the broadcast field be nullable ? In the BroadcastPicture ?
There was a problem hiding this comment.
Yes, in order to not use the Broadcast.loading()
There was a problem hiding this comment.
But why not keep it ? It is easier to create the shimmer like this.
I can also do it with defining a nullable broadcast field in the BroadcastPicture but I am not sure that this will make the code cleaner.
There was a problem hiding this comment.
Ok let's use a dummy Broadcast in the BroadcastPicture if it's easier, but don't define the factory constructor here; we don't want it to be used elsewhere.
Just construct the dummy one directly in the view (and you can use a const).
There was a problem hiding this comment.
I can't use a const because DateTime doesn't have a const constructor
|
I added a delay of 5 seconds to the request in order to have time to see the loading behavior when reaching next page |
| const BroadcastPicture({super.key, required this.broadcast}); | ||
| const BroadcastPicture({required this.broadcast}); | ||
|
|
||
| BroadcastPicture.loading() : broadcast = Broadcast.loading(); |
There was a problem hiding this comment.
It could use a const I think.
There was a problem hiding this comment.
I can't because the value I assign to broadcast is not a const value due to the presence of Datetime in Broadcast
| BroadcastRound? get curentRound => | ||
| rounds.where((r) => r.status == RoundStatus.live).firstOrNull ?? | ||
| rounds.where((r) => r.status == RoundStatus.finished).lastOrNull; | ||
| factory Broadcast.loading() => Broadcast( |
There was a problem hiding this comment.
Ok let's use a dummy Broadcast in the BroadcastPicture if it's easier, but don't define the factory constructor here; we don't want it to be used elsewhere.
Just construct the dummy one directly in the view (and you can use a const).
5 seconds seems a lot. Can you try with 2 seconds? |
I added it only for the video so we can see better what happens with the shimmer. Do you want me to record again with 2 seconds instead of 5 ? |
Ah ok I didn't get it was for the recording. No I thought an artificial delay was needed (it sometimes is) to avoid "blinking" due to shimmer loading. If that is the case I'd say 1-2s max is enough. |
|
Hey guys, web has a row or two of smaller detail text below each of the larger tournament titles. Seems like they're still experimenting with what to show on the tournament posters but these have included: There is also a red icon that shows # of viewers Would it make sense to have a list vs grid switcher (using images of Trakt apps implementation below for reference) so that you can alternate between 2 vs 1 title per row. With just 1 expanded title per row, at least some of that underlying detail can be shown. At least some of that detail is important context whereby I think I'd personally keep it at 1 title per row if given the option.
And secondly, do you think the viewer count in red (see very first photo above) is worth showing in the app as well? |
I don't have this problem on my side |
It works as anonymous, but if I log in, it stops to work. Does it also work for you when you're logged in? |
How can I try the broadcast feature on a local lila to be able to log in ? I think there is nothing to populate the database with broadcast. |
recording.webm