amugofjava / anytime_podcast_player

Simple, easy to use Podcast player app written in Flutter and Dart.
BSD 3-Clause "New" or "Revised" License
377 stars 99 forks source link

Add Show Funding Menu settings to show/hide FundingMenu on PodcastDetails #32

Closed erdemyerebasmaz closed 3 years ago

erdemyerebasmaz commented 3 years ago

Since we have our own payment system integrated into the plugin via hooks, this raised the need to hide FundingMenu on PodcastDetails page. We believe the least intrusive way to go with this change was to add a showFunding flag to AppSettings.

This PR introduces showFunding settings flag that allows developers to hide FundingMenu on PodcastDetails page. This setting is true by default and won't be configurable through Settings page.

amugofjava commented 3 years ago

Hi @erdemyerebasmaz,

Would a podcast not potentially support both Lightning and Funding links, or are you presenting the funding links within Breez itself?

kingonly commented 3 years ago

@amugofjava only Lightning is supported in Breez

amugofjava commented 3 years ago

Hi, I've made a slight adjustment to podcast details. Returning a SizedBox whilst waiting for data to appear in the stream causes a noticeable screen jump as you can see in the first video below (slowed down to make it more visible). It shouldn't have any impact on the funding button, but let me know if you have any problems.

https://user-images.githubusercontent.com/5526902/111432567-17d6e180-86f5-11eb-8249-befc3be57f47.mp4

https://user-images.githubusercontent.com/5526902/111432599-1f968600-86f5-11eb-9689-143337c72c41.mp4

erdemyerebasmaz commented 3 years ago

Hi, I've made a slight adjustment to podcast details. Returning a SizedBox whilst waiting for data to appear in the stream causes a noticeable screen jump as you can see in the first video below (slowed down to make it more visible). It shouldn't have any impact on the funding button, but let me know if you have any problems.

settings_jank.mp4 settings_fixed.mp4

Good catch! @amugofjava I tested and unfortunately funding button is displayed till showFunding setting is retrieved. because stream is initialized with AppSettings.sensibleDefaults()

https://user-images.githubusercontent.com/4012752/111447351-6a78c380-871e-11eb-9c4c-8d63fac02d4b.mp4

What we should do is to move StreamBuilder inside Column children where FundingMenu is:

StreamBuilder<AppSettings>(
    stream: _settingsBloc.settings,
                ⋮
        child: [
        SubscriptionButton(podcast),
        PodcastContextMenu(podcast),
        settings.showFunding ? FundingMenu(podcast.funding) : Container(),
        ]
                ⋮
SubscriptionButton(podcast),
PodcastContextMenu(podcast),
StreamBuilder<AppSettings>(
   stream: _settingsBloc.settings,
   builder: (context, settingsSnapshot) {
      return (settingsSnapshot.hasData && settingsSnapshot.data.showFunding)
                 ? FundingMenu(podcast.funding)
                 : Container();
      },
),

This way the wait won't affect the whole widget.

erdemyerebasmaz commented 3 years ago

@amugofjava Please see my comment above.

amugofjava commented 3 years ago

Hi @erdemyerebasmaz,

Yes, I'll give that a try and see if it improves things.

amugofjava commented 3 years ago

Hi @erdemyerebasmaz,

I tried moving the stream closer to the widget using it. It's good practice but made no difference. So, I have changed the settings BLoC to allow direct access to the current settings. This does break the 'contract' that BLoCs should only be accessed via sinks and streams, but in this case I think it's more important to have a jank free ui! :) I hadn't noticed until I saw your video that the play button was janky too so I have also fixed that.