-
Notifications
You must be signed in to change notification settings - Fork 356
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
Cascading Auto Refresh for dialog fields #264
Conversation
https://www.pivotaltracker.com/story/show/138457551 Instead of limiting the fields that get refreshed to the current tab, we will go from top to bottom in the current tab, and then move on to the next tab. There is also no longer a multidimensional array, just an array of refreshable objects
Small correction with the latest update, this is no longer true and tabs will continue to be affected by auto-refreshes on other tabs, processing from the leftmost tab to the rightmost tab. |
👍 Wow, this is really cool!! Looks all good to me. |
@eclarizio - I like it. 👍 |
@eclarizio Overall this is looking good. The one thing I am not seeing at the moment is an auto-refresh triggered from a text field. I modified each text field on the second tab and no automate methods were run. Three of the text fields are configured to auto-refresh other fields. The other fields seem to be working as expected. |
var thisIsTheFieldToUpdate = function(event) { | ||
tabIndex = event.data.tabIndex; | ||
groupIndex = event.data.groupIndex; | ||
fieldIndex = event.data.fieldIndex; |
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.
Need the var
declaration for tabIndex
, groupIndex
and fieldIndex
@@ -135,6 +143,7 @@ var dialogFieldRefresh = { | |||
$('.dynamic-text-box-' + fieldId).val(responseData.values.text); | |||
dialogFieldRefresh.setReadOnly($('.dynamic-text-box-' + fieldId), responseData.values.read_only); | |||
dialogFieldRefresh.setVisible($('#field_' +fieldId + '_tr'), responseData.values.visible); | |||
callback.call(); | |||
}; | |||
|
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.
If easily doable, can you DRY up refreshTextAreaBox
and refreshTextBox
since both functions are so identical?
https://www.pivotaltracker.com/story/show/138457551 This ensures that the triggering of auto-refreshes for text boxes and other fields that are relying on the observe javascript have the current_index to compare against to figure out which field is next to refresh.
@gmcculloug Oops, should be fixed now, text boxes were behaving slightly differently due to how the observe functionality works. @AparnaKarve Good catch on the var declaration. I ideally would've liked to refactor all of those 'done' functions since they're very similar, but you're right in that the text ones were identical aside from the selector so I've pulled that out. Thanks. |
Checked commits eclarizio/manageiq-ui-classic@72a728f~...e68866d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 app/helpers/application_helper/dialogs.rb
app/views/shared/dialogs/_dialog_field.html.haml
spec/helpers/application_helper/dialogs_spec.rb
|
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 @eclarizio! Looks good now.
Retested and the text fields are working now, thanks. Looks good. |
Backported to Darga via ManageIQ/manageiq#13778 |
Backported to Euwe via ManageIQ/manageiq#13779 |
This changes the functionality of auto-refreshing dialog fields a little bit. Here's how they worked before:
If multiple fields were set to 'auto-refresh', any time another field that was set to 'trigger auto-refresh', all of those fields would refresh at the same time, and that would be the end of the chain. Sometimes, people were relying on one field being refreshed before the next, and this approach wouldn't work.
After this change, the triggers are processed from top to bottom, and any field that is set to 'auto-refresh' will auto-refresh, and then trigger the next field (if it's set to trigger) and so-on, until the end.
of the tab. Tabs will no longer trigger auto-refreshes between each other like they used to.https://www.pivotaltracker.com/story/show/138457551
/cc @gmcculloug
You can use this
dialog_export_20170126.yml.zip (I had to zip it up cause github won't allow yml files) for a sample dialog I was testing around with that includes all of the different types of dialog fields we have and some that depend on others.
You can use this datastore_2017_01_26.zip to get the automate methods that I used in the above dialog. The only relevant methods are in the "CascadingAutoRefresh" domain. Ignore the other domains, they're just what I had in my db.