daattali / shinyalert

🗯️ Easily create pretty popup messages (modals) in Shiny
https://daattali.com/shiny/shinyalert-demo/
Other
241 stars 26 forks source link

First instance of an alert doesnt center correctly on screen. Subsequent alerts all position correctly #74

Open gwiesner opened 1 year ago

gwiesner commented 1 year ago

Noticed this issue after updating RStudio to Chrerry Blossom and updating Shiny and Shinyalerts packages to latest versions: image

gwiesner commented 1 year ago

Updated R from version 4.0.2 to 4.2.3 and the issue is fixed

daattali commented 1 year ago

I've noticed this issue too sometimes, to me it happens non-deterministically but only with the vertical alignment, not the horizontal alignment. I suspect that for some reason one of the resources isn't loaded quickly enough, but I don't know which and why.

One way to guarantee this doesn't happen is to pre-load the shinyalert dependencies by adding useShinyalert(force = TRUE) anywhere in the UI (note the force=TRUE!).

I just made a change that may or may not fix it, it's not pretty, but it might work. Can you try to install the package from the fix-74 branch, and let me know if you're still seeing this issue? You can install using

remotes::install_github("daattali/shinyalert", "fix-74")
daattali commented 1 year ago

I had this page open from last night and didn't notice that you closed the issue.

Is the issue really fixed for you when you upgraded R? It's persistently fixed? I'm on R 4.2.1 and occassionally see this issue

daattali commented 1 year ago

@gwiesner can you confirm if the issue was really solved without my fix?

gwiesner commented 1 year ago

Hi Dean,

I can confirm no issues since updating R.

Thanks.


From: Dean Attali @.> Sent: Tuesday, 18 April 2023, 2:22 pm To: daattali/shinyalert @.> Cc: Glen Wiesner @.>; Mention @.> Subject: Re: [daattali/shinyalert] First instance of an alert doesnt center correctly on screen. Subsequent alerts all position correctly (Issue #74)

@gwiesnerhttps://github.com/gwiesner can you confirm if the issue was really solved without my fix?

— Reply to this email directly, view it on GitHubhttps://github.com/daattali/shinyalert/issues/74#issuecomment-1512413991, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALGWUR53ZWXLOANPOVDDW7TXBYJJVANCNFSM6AAAAAAW5YZNYQ. You are receiving this because you were mentioned.Message ID: @.***>

gwiesner commented 1 year ago

Apologies Dean. The issue is still there. Noticed today when using laptop (less likely to notice on larger monitor)


From: Gee Spang @.> Sent: Tuesday, 18 April 2023, 2:44 pm To: daattali/shinyalert @.> Subject: Re: [daattali/shinyalert] First instance of an alert doesnt center correctly on screen. Subsequent alerts all position correctly (Issue #74)

Hi Dean,

I can confirm no issues since updating R.

Thanks.


From: Dean Attali @.> Sent: Tuesday, 18 April 2023, 2:22 pm To: daattali/shinyalert @.> Cc: Glen Wiesner @.>; Mention @.> Subject: Re: [daattali/shinyalert] First instance of an alert doesnt center correctly on screen. Subsequent alerts all position correctly (Issue #74)

@gwiesnerhttps://github.com/gwiesner can you confirm if the issue was really solved without my fix?

— Reply to this email directly, view it on GitHubhttps://github.com/daattali/shinyalert/issues/74#issuecomment-1512413991, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALGWUR53ZWXLOANPOVDDW7TXBYJJVANCNFSM6AAAAAAW5YZNYQ. You are receiving this because you were mentioned.Message ID: @.***>

daattali commented 1 year ago

Is the issue still there after installing from the fix branch?

daattali commented 1 year ago

@gwiesner can you please verify if the fix at https://github.com/daattali/shinyalert/tree/fix-74 solves your problem?

ch0c0l8ra1n commented 10 months ago

Trivial non-hacky fix with css flexbox and a minor change to layout @daattali

Should I submit a PR?

image
daattali commented 10 months ago

Could you show me the code? If it's indeed trivial and clean, and does not cause any regressions, then it would be very welcomed.

ch0c0l8ra1n commented 10 months ago

@daattali The fix involves changing sweetalert.min.css and sweetalert.min.js

I can modify a development bundle (instead of the minified one) and send a pull request and I suppose you can manage what you do with the rest? Or you could make a branch with the development bundles yourself and I could send a pull request with the fixed code to that branch instead.

daattali commented 10 months ago

Is it possible to include the changes in a different css/js, not the official ones that are pegged to a specific version? I don't love the idea of taking a released versioned library file and making small changes to it

shahreyar-abeer commented 9 months ago

I have an app with a {shinyalert} and was reported the same issue. No specific reasons or settings. Seems to occur randomly.

ch0c0l8ra1n commented 8 months ago

@daattali I have sent a PR that changes the cumbersome top/left alignment with flexbox.

ch0c0l8ra1n commented 8 months ago

@daattali in retrospect you should probably just upgrade the sweet alert version. Sweet Alert 2 (at least) uses idiomatic css with flex boxes and grids.

daattali commented 5 months ago

I've looked at sweetalert v2 in the past and its syntax would not have worked. I didn't realize that there is a sweetalert2 (that's different from sweetalert v2) - briefly going through their docs looks like it could work. I'll try that. https://github.com/daattali/shinyalert/issues/84