-
Notifications
You must be signed in to change notification settings - Fork 301
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
HPCC-24341 Dismiss dialog and clear fields #13919
HPCC-24341 Dismiss dialog and clear fields #13919
Conversation
https://track.hpccsystems.com/browse/HPCC-24341 |
@GordonSmith Please review. |
@@ -51,7 +52,9 @@ define([ | |||
|
|||
// Hitched actions --- | |||
_onSave: function (event) { | |||
var dialog = dijit.byId("stubUserDialog"); | |||
var context = this; |
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.
The majority of the work is done here where I pass in the widget as a param when initialized since it is undefined. As well as clearing the fields if exception is thrown.
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.
Did HPCCPlatformWidget.js actually change - lots of white space changes?
Yes, #362 the eslint format on save must have fired off white space changes. |
if (!userInfo.init({ Username: this.userName })) { | ||
if (!userInfo.init({ | ||
Username: this.userName, | ||
Widget: userDialog |
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.
Passed in param here.
<input name="newPasswordRetype" type="password" required="true" pwtype="verify" data-dojo-props="invalidMessage:'${i18n.PasswordsDoNotMatch}', placeHolder:'${i18n.MustContainUppercaseAndSymbol}'" data-dojo-type="dijit.form.ValidationTextBox" /> | ||
<input name="newPasswordRetype" type="password" required="true" pwtype="verify" | ||
data-dojo-props="invalidMessage:'${i18n.PasswordsDoNotMatch}', placeHolder:'${i18n.MustContainUppercaseAndSymbol}'" | ||
data-dojo-type="dijit.form.ValidationTextBox" /> |
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 like you have enabled a HTML formatter? Can you roll back these changes (or adjust your formatter to not split the lines?
- Dismissed dialog due to an undefined widget - Cleared input fields if passwords did not meet criteria Signed-off by: Miguel Vazquez <miguel.vazquez@lexisnexisrisk.com>
@GordonSmith modified IDE to not format on save for selected options. |
Automated Smoketest: ✅
|
Automated Smoketest: ✅
|
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 ok - one question re the potential reuse?
@@ -51,7 +53,9 @@ define([ | |||
|
|||
// Hitched actions --- | |||
_onSave: function (event) { | |||
var dialog = dijit.byId("stubUserDialog"); | |||
var context = this; | |||
var dialog = this.params.Widget; |
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.
Passing an entire Widget via params will mean that this page cannot be displayed on its own or in a react component (which may be ok?)
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 envision us creating a dialog component that we pass in ...props
to replace items such as these in the current world.
@ghalliday good to pull |
Reviewed-by: Gavin Halliday ghalliday@hpccsystems.com |
Signed-off by: Miguel Vazquez miguel.vazquez@lexisnexisrisk.com
Type of change:
Checklist:
Smoketest:
Testing: