-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Tiny improvement on the getShouldShowAlertPrompt method #5738
Conversation
@@ -107,7 +107,7 @@ class WorkspaceInvitePage extends React.Component { | |||
* @returns {Boolean} | |||
*/ | |||
getShouldShowAlertPrompt() { | |||
return _.size(lodashGet(this.props.policy, 'errors', {})) > 0 || lodashGet(this.props.policy, 'alertMessage.length') > 0; | |||
return _.size(lodashGet(this.props.policy, 'errors', {})) > 0 || lodashGet(this.props.policy, 'alertMessage').length > 0; |
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.
I think this will actually cause the same problem as before 😅. If alertMessage isn't there it'll be calling length on undefined. I think we need just a default value to be the third parameter of the lodashGet function.
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.
Ha, I was just testing this and thought "wait a minute....." 🤦
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.
Gimme one sec. Running this with a new account. Missed this comment #5727 (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.
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.
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by @TomatoToaster in version: 1.1.7-25 🚀
|
🚀 Deployed to production by @roryabraham in version: 1.1.8-9 🚀
|
Details
Implementing a tiny improvement that @marcaaron suggested here: #5729 (comment)
Related to
#5729 (comment)
Tests
Same as Web QA done locally
QA Steps
Tested On
Screenshots
Web