-
Notifications
You must be signed in to change notification settings - Fork 54
Auth: Display authorization denied message to the user access is denied #2238
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
Auth: Display authorization denied message to the user access is denied #2238
Conversation
…ation process when access is denied.
📊 Performance Test ResultsComparing 191af6d vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
ivan-ottinger
left a comment
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.
Nice improvement! The code looks good. I have left a copy suggestion.
epeicher
left a comment
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.
Thanks for improving this @gavande1! I have tested it, and I can see now the new message, which sounds less scary. I agree with the suggestion from @ivan-ottinger, but overall this LGTM!
katinthehatsite
left a comment
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.
Looks good to me 👍 I left some suggestions for the copy.
…ation process when access is denied.
Related issues
Proposed Changes
Testing Instructions
npm startPre-merge Checklist