Skip to content
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 select2 value updation for crmAddName Directive #11979

Merged
merged 1 commit into from
Apr 18, 2018

Conversation

deb1990
Copy link
Contributor

@deb1990 deb1990 commented Apr 16, 2018

Overview

In ang/crmCaseType.js, crmAddName directive is present. But if the scope variable mentioned in crm-options is updated with new values, the select2 dropdown does not get updated with new values.

This PR fixes the problem.

Before

The dropdown values does not update

After

The dropdown values updates

Technical Details

How to reproduce?

  1. Go to https://github.com/civicrm/civicrm-core/blob/master/ang/crmCaseType.js#L128
  2. And update the code in the following way
crmCaseType.controller('CaseTypeCtrl', function($scope, crmApi, apiCalls, $timeout) {
    ...
    $scope.activityTypeOptions = _.map(apiCalls.actTypes.values, formatActivityTypeOption);
    $timeout(function () {
      console.log('Scope variable updated');
      $scope.activityTypeOptions = [];
    }, 1000);
    ...
  1. Open browser, see that in console 'Scope variable updated' is printed but the dropdown values are not updated.

Problem
Previously the code used to update the the select2 was $(input).select2('data', scope[attrs.crmOptions]);, but it did not work.

Solution
Using a function instead of a variable inside the data object, solved the problem
Source: https://stackoverflow.com/a/17348414/2400594

 data: function () {
   return { results: scope[attrs.crmOptions] };
 }

@civicrm-builder
Copy link

Can one of the admins verify this patch?

@deb1990 deb1990 force-pushed the fix-crm-add-name-directive branch from 8cd66ad to 91dc528 Compare April 16, 2018 12:53
@deb1990 deb1990 changed the title (WIP)Fix crm add name directive [WIP] Fix crm add name directive Apr 16, 2018
@deb1990 deb1990 changed the title [WIP] Fix crm add name directive Fix crm add name directive Apr 16, 2018
@deb1990 deb1990 changed the title Fix crm add name directive Fix select2 value updation for crmAddName Directive Apr 16, 2018
@deb1990 deb1990 force-pushed the fix-crm-add-name-directive branch from 91dc528 to c0bb8bd Compare April 16, 2018 13:07
@eileenmcnaughton
Copy link
Contributor

@monishdeb @colemanw one for you guys to look at

@colemanw
Copy link
Member

@civicrm-builder add to whitelist

@colemanw
Copy link
Member

Hey @deb1990 thanks for the contribution! Do you have any idea why the existing $watch function was not working? Glancing at the code it certainly looks like it ought to have worked.

@colemanw
Copy link
Member

To answer my own question, I've run into this type of thing before and the answer was to either pass true for the 3rd param to $watch or else use $watchCollection. Did you try either of those? Your function solution is a good one and may be the right fix, but I'm just curious to know why things were broken.

@deb1990
Copy link
Contributor Author

deb1990 commented Apr 18, 2018

@colemanw The watch function was not the problem, as the watcher was getting fired correctly whenever values are changed, but the problem was with $(input).select2('data', scope[attrs.crmOptions]); statement, as this does not work at all.
After doing some googling I found that the correct way to update data is different.

@colemanw
Copy link
Member

colemanw commented Apr 18, 2018

@colemanw colemanw merged commit 671aab6 into civicrm:master Apr 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants