-
Notifications
You must be signed in to change notification settings - Fork 650
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.
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
Fix notification warning positioning issue #14920
Conversation
cc: @ana-md. That banner should no longer be there, right? Thank you! |
It should appear only for the free users that are being migrated today, it appears only for the users scheduled each day (infra takes care of that). The one in staging was only a test we did to make sure everything was working, so I guess that one doesn't need to be there. @gonzalosr can you confirm this please? |
Yes, but we are mixing things a little bit. Regardless of the message we are showing right now (PG11 migration) we have a warning component that we use to make important announcements to the user. The main problem here is that the same message is shown in two different banners. We are working so that the banner below does not appear when it doesn’t have to. |
@ana-md yes, the staging one doesn't need to be there anymore |
But please @gonzalosr, don't remove it because I need it to test all the cases!! 🙏 |
@alejandraarri Thought so! don't worry :) |
Thank you all! Keep going @alejandraarri! |
Opened a new issue to fix the second error (#14936) so we can merge this PR and correct the positioning issue as it is important for the new Professional Plan release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Such a great improvement!
Acceptance steps: Log in as staging user (with maintenance warning banner active):
Log in as staging user (with maintenance warning banner inactive):
Acceptance 🆗 |
This PR solves 2 problems with the notification warning.
Related issue
#14859