0

I have a table of read-only phone numbers. A phone number has a number and a type (e.g. Mobile or Home). Each phone number has an "Edit" button that, when clicked, allows you to edit the phone number and type in a modal dialog. I'm using Knockoutjs for the read-only table and the editor. The table binds to an observableArray of PhoneVMs and the editor works on a single PhoneVM. Because I want the user to have to click OK on the modal before any of their changes are applied, the modal works on a copy of the selected PhoneVM and when they click OK, it replaces the originally clicked PhoneVM in the observableArray that the table is bound to. That's all working great.

Now I have a need to allow the first phone in the list to be edited on the same page as the read only table (without a modal). The idea is for it to be easier to enter the first phone earlier in the workflow. So you would enter your phone on the page and it would automatically appear in the read only list below where you could also edit it in the modal as normal. I thought Knockout would make this easy but I hit a snag. From this point it will be easier to just show an example of what is going wrong. Do the following in this fiddle: https://jsfiddle.net/ph4mhsof/

  1. Edit the phone number and tab out of the textbox. Notice the first phone in the All Phones list updates too.
  2. Change the Phone Type in the dropdown. Notice both the Type ID and the Type Name change appropriately in the All Phones table.
  3. Click Remove First Phone. The 2nd phone becomes the new first phone.
  4. Edit the phone number and tab out of the textbox. Notice the first phone in the All Phones list updates as expected.
  5. Change the Phone Type in the dropdown. Notice only the Type ID updates in the All Phones list. The Type Name does not update.

I am using a custom binding to bind the type name to the select's text. It seems the valueAccessor in that binding's init function must be pointing specifically to the original first PhoneVM's PhoneTypeName property but what I need it to do is point to the firstPhone computed property's PhoneTypeName. Is there any way to fix this?

Copy of original jsfiddle:

function phoneListVM() {
  var _self = this;
  this.phones = ko.observableArray([
    new phoneVM(1, "Mobile", "123-234-3456"),
    new phoneVM(2, "Home", "654-343-3211")
  ]);

  this.firstPhone = ko.computed(function() {
    return _self.phones()[0];
  });
}

function phoneVM(typeID, typeName, Number) {
  this.PhoneTypeID = ko.observable(typeID);
  this.PhoneTypeName = ko.observable(typeName);
  this.PhoneNumber1 = ko.observable(Number);
}

ko.bindingHandlers.selectedTextValue = {
  init: function(element, valueAccessor) {
    var value = valueAccessor();

    $(element).change(function() {
      value($("option:selected", this).text());
    });
  },
  update: function(element, valueAccessor) {}
};

$(document).ready(function() {
  var phoneList = new phoneListVM()
  ko.applyBindings(phoneList);
  $("button").click(function() {
    phoneList.phones.shift();
  });
});
.editor{
  background-color:rgba(200,200,250, 0.2);
  border: 1px solid rgba(0,0,0, 0.2);
  padding: 10px;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/knockout/3.4.2/knockout-min.js"></script>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>

<h4>
Edit First Phone
</h4>
<div class="editor">
  <p>Phone Number:
    <input data-bind="value: firstPhone().PhoneNumber1" />
  </p>
  <p>Phone Type:
    <select data-bind="value:firstPhone().PhoneTypeID, selectedTextValue:firstPhone().PhoneTypeName">
      <option value="0"></option>
      <option value="1">Mobile</option>
      <option value="2">Home</option>
    </select>
  </p>
</div>

<h4>
All Phones
</h4>
<table>
  <thead>
    <tr>
      <th>Type ID</th>
      <th>Type Name</th>
      <th>Number</th>
    </tr>
  </thead>
  <tbody data-bind="foreach:phones">
    <tr>
      <td><span data-bind="text:PhoneTypeID"></span></td>
      <td><span data-bind="text:PhoneTypeName"></span></td>
      <td><span data-bind="text:PhoneNumber1"></span></td>
    </tr>
  </tbody>
</table>
<button type="button">
  Remove First Phone
</button>
xr280xr
  • 12,621
  • 7
  • 81
  • 125

3 Answers3

0

In my opinion the custom binding is overkill. I've updated your code to a little with some methods and a dedicated selected observable so you always know which phone is selected.

Let me know if that's what you were looking for.

function phoneListVM() {
  var _self = this;
  this.phones = ko.observableArray([
   // Removed typeName
    new phoneVM(1, "123-234-3456"),
    new phoneVM(2, "654-343-3211")
  ]);
  
  // Observable to see which phone is currently selected
  this.SelectedPhone = ko.observable(_self.phones().length > 0 ? _self.phones()[0] : '');
  
  // Allow editing whichever phone they want
  this.EditPhone = function(obj) {
    _self.SelectedPhone(obj);
  };
  
  // Remove first phone and check if there are any more phones, if so add it to the selected phone
  this.RemoveFirstPhone = function() {
    var firstPhone = _self.phones()[0];
    if(firstPhone) {
        _self.phones.remove(firstPhone);
        _self.SelectedPhone(_self.phones().length > 0 ? _self.phones()[0] : '');
    }
  }
}

// Removed typeName and made it computed. Could be replaced with some lodash _.find if you are storing an array of types in the global space
function phoneVM(typeID, Number) {
  var self = this;
  this.PhoneTypeID = ko.observable(typeID);
  this.PhoneNumber1 = ko.observable(Number);
  this.PhoneTypeName = ko.computed(function() {
    switch (self.PhoneTypeID().toString()) {
      case '1':
        return 'Mobile';
        break;
      case '2':
        return 'Home';
        break;
    }
  });
}
$(document).ready(function() {
  var phoneList = new phoneListVM()
  ko.applyBindings(phoneList);
});
.editor {
  background-color: rgba(255, 255, 255, 0.7);
  border: 1px solid rgba(0, 0, 0, 0.2);
  padding: 10px;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/knockout/3.4.2/knockout-min.js"></script>
<h4>
Edit First Phone
</h4>
<div class="editor" data-bind="with: SelectedPhone, visible: SelectedPhone()">
  <p>Phone Number:
    <input data-bind="value: PhoneNumber1" />
  </p>
  <p>Phone Type:
    <select data-bind="value:PhoneTypeID">
      <option value="0"></option>
      <option value="1">Mobile</option>
      <option value="2">Home</option>
    </select>
  </p>
</div>

<h4>
All Phones
</h4>
<table>
  <thead>
    <tr>
      <th>Type ID</th>
      <th>Type Name</th>
      <th>Number</th>
    </tr>
  </thead>
  <tbody data-bind="foreach:phones">
    <tr>
      <td><span data-bind="text:PhoneTypeID"></span></td>
      <td><span data-bind="text:PhoneTypeName()"></span></td>
      <td><span data-bind="text:PhoneNumber1"></span></td>
      <td>
        <button data-bind="click: $root.EditPhone">
          Edit
        </button>
      </td>
    </tr>
  </tbody>
</table>
<button type="button" data-bind="click: RemoveFirstPhone, visible: phones().length > 0">
  Remove First Phone
</button>
Budhead2004
  • 579
  • 4
  • 10
0

You're probably over thinking this. There's no need to create a custom binder. You can use knockout's options binding.

// create an array
var phoneTypes = [{
  text: "Mobile",
  value: 1
}, {
  text: "Home",
  value: 2
}];

function phoneListVM() {
  var _self = this;
  // this will be bound to the dropdown
  _self.phoneTypes = ko.observableArray(phoneTypes)
  this.phones = ko.observableArray([
    new phoneVM(1, "Mobile", "123-234-3456"),
    new phoneVM(2, "Home", "654-343-3211")
  ]);

  this.firstPhone = ko.computed(function() {
    return _self.phones()[0];
  });
}

function phoneVM(typeID, typeName, Number) {
  var self = this;

  this.PhoneTypeID = ko.observable(typeID);
  this.PhoneNumber1 = ko.observable(Number);

  // get the value from the phonetypes array using the PhoneTypeID
  self.PhoneTypeName = ko.computed(function() {
    var type = phoneTypes.filter(function(a) {
      return a.value === self.PhoneTypeID()
    });
    return type.length > 0 ? type[0].text : undefined;
  })
}

And change the HTML to:

<select data-bind="options: phoneTypes,
                   optionsText: 'text',
                   optionsValue: 'value',
                   optionsCaption: 'Choose',
                   value: firstPhone().PhoneTypeID">
</select>

You can have a complex object as selected value in knockout. So you could have a PhoneType property in phoneVM and bind the 2 properties of PhoneType to the text.

Here's an updated fiddle


I didn't understand a great deal about why you are allowing editing only on the first option or how would a user edit the second option. But, you can take a look at this fiddle on how to make each item editable in a list of items.


Update after comments:

Even if the select isn't created by knockout's bindings, you still wouldn't need custom binding. You can make the PhoneTypeName a computed property like I suggested before and get the text from the options based on the PhoneTypeID.

function phoneVM(typeID, typeName, Number) {
  var self = this;
  this.PhoneTypeID = ko.observable(typeID);
  this.PhoneNumber1 = ko.observable(Number);

  self.PhoneTypeName = ko.computed(function() {
    var type = $('#select option[value='+self.PhoneTypeID() +']');
    return type.length > 0 ? type.text() : undefined;
  });
}

Here's an updated fiddle

The reason your change event wasn't getting fired is probably because the element you add the event to is now removed from the DOM.

adiga
  • 34,372
  • 9
  • 61
  • 83
  • Thanks! This is a nice solution, but my dropdown is currently generated server-side (asp.net mvc) and isn't using knockout. I was trying to avoid the complexity of using knockout with another input that is also bound on the server-side. To try to clarify here is an example of the similar base functionality I have (the part I said is working great): https://demos.telerik.com/aspnet-mvc/grid/editing-popup. I'm trying to add, in addition to that, the ability to enter the first item directly on the page (it does not have a delete button, fyi). My fiddle only shows the problem, not my functionality – xr280xr Oct 17 '17 at 22:20
  • @xr280xr I have updated the answer accordingly. Test it out and let me know. – adiga Oct 18 '17 at 08:01
0

I agree with the other two answers that customBinding is overwork. But as you said that you can't easily change the code, I will show you your problem.

In your custom binding declaration, you've just defined the init function but left update function blank. That's the problem. When you make change on the select box, the change event is fired but there is no event handler, so there's nothing happened.

Infact, your custom binding has added change event handler to the select box successfully. That's why before removing the first phone number, everything's ok. But the event handler is removed when you remove the first phone number, because you are able to add only one event handler to select box for change event.

The solution is: Leave your init function blank and move all the current init function contents to the update function as below.

ko.bindingHandlers.selectedTextValue = {
  init: function(element, valueAccessor) {

  },
  update: function(element, valueAccessor) {
    var value = valueAccessor();

    $(element).change(function() {
      value($("option:selected", this).text());
    });
  }
};
trgiangvp3
  • 253
  • 3
  • 9