Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
gwiesner opened this issue Apr 14, 2023 · 16 comments
Open

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

gwiesner opened this issue Apr 14, 2023 · 16 comments
Labels
bug help wanted

Comments

@gwiesner
Copy link

Noticed this issue after updating RStudio to Chrerry Blossom and updating Shiny and Shinyalerts packages to latest versions:
image
@gwiesner
Copy link
Author

gwiesner commented Apr 14, 2023 •
edited
Loading

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

daattali added a commit that referenced this issue Apr 14, 2023
@daattali
try to hackily work around #74
a2e40f3
@daattali
Copy link
Owner

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
Copy link
Owner

daattali commented Apr 14, 2023 •
edited
Loading

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 daattali reopened this Apr 14, 2023
@daattali
Copy link
Owner

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

@gwiesner
Copy link
Author

gwiesner commented Apr 18, 2023 via email

@gwiesner
Copy link
Author

gwiesner commented Apr 18, 2023 via email

@daattali
Copy link
Owner

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

@daattali
Copy link
Owner

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

@ch0c0l8ra1n
Copy link
Contributor

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

Should I submit a PR?

image

@daattali
Copy link
Owner

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
Copy link
Contributor

@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
Copy link
Owner

daattali commented Nov 7, 2023

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
Copy link

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

@ch0c0l8ra1n
Copy link
Contributor

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

@ch0c0l8ra1n
Copy link
Contributor

@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
Copy link
Owner

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. #84

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@daattali @shahreyar-abeer @ch0c0l8ra1n @gwiesner