LeoNatan / LNPopupUI

A SwiftUI library for presenting views as popups, much like the Apple Music and Podcasts apps.
MIT License
320 stars 29 forks source link

Possible regression between 1.4.0 and 1.4.1 in popupBarItems #21

Closed michaelschwinges closed 1 year ago

michaelschwinges commented 1 year ago

Describe the Bug

When updating LNPopupView from 1.4.0 to 1.4.1, then the frame of popupBarItems' Button moves around.

To Reproduce

Change SPM from exact version 1.4.0 to exact version 1.4.1, then the behaviour manifests itself.

Expected Behavior

I guess I would expect the LoadingSpinnerView which is the button's content not to shift around like it does - while there is a rotation modifier on a view within that view, it does not contain anything that changes position or frame size. My workaround for now will be to stay on 1.4.0 but this isn't viable for when I ship, as LNPopupUI is maintained and will change and fix and improve over time leaving me behind.

Screenshots

Behaviour on 1.4.0:

https://user-images.githubusercontent.com/11598368/222247043-28815db1-5475-42e4-aebb-dbc9d14748e6.mov

Behaviour on 1.4.1:

https://user-images.githubusercontent.com/11598368/222247142-11811252-f4f5-4f6d-8bb2-25a77c7ec5c6.mov

Additional Context

I am changing the button's content when the play state changes as follows:

.popupBarItems({
    Button(action: {
        playerViewModel.togglePlaying()
    }) {
         if playerViewModel.playerState == .loading {
            LoadingSpinnerView()
                 .frame(width: 35, height: 35)
                 .accentColor(.primary)
                 .padding(2)
                 .border(.pink)
        } else if playerViewModel.playbackState == .stopped || playerViewModel.playbackState == .paused {
            Image(systemName: "play.fill")
                .renderingMode(.template)
                .foregroundColor(.primary)
                .font(.title2)
                .padding(10)
        } else if playerViewModel.playbackState == .playing {
            Image(systemName: "pause.fill")
                .renderingMode(.template)
                .foregroundColor(.primary)
                .font(.title2)
                .padding(10)
        }
    }
})
LeoNatan commented 1 year ago

I will take a look soon. Sorry for the delay!

LeoNatan commented 1 year ago

Could you please post the source for LoadingSpinnerView, or a self-contained example, where this issue reproduces? Thanks

LeoNatan commented 1 year ago

Ping @michaelschwinges

LeoNatan commented 1 year ago

Closing this issue. If you post the example project, I’ll reopen and look into it.