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
Fixed contact update after propertyGroup refactor #425
Conversation
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Codecov Report
@@ Coverage Diff @@
## master #425 +/- ##
=======================================
Coverage 16.62% 16.62%
=======================================
Files 61 61
Lines 1311 1311
=======================================
Hits 218 218
Misses 1093 1093
Continue to review full report at Codecov.
|
templates/contactDetails.html
Outdated
@@ -42,7 +42,7 @@ | |||
</div> | |||
</header> | |||
<section> | |||
<propertyGroup ng-repeat="prop in ctrl.contact.props | toArray | orderDetailItems:'$key'" model="ctrl.contact" data="prop" name="prop.$key"></propertyGroup> | |||
<propertyGroup ng-repeat="prop in ctrl.contact.props | toArray | orderDetailItems:'$key'" model="ctrl.contact" addressBook="ctrl.addressBook" data="prop" name="prop.$key"></propertyGroup> |
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.
@Henni this doesn't work and fails with a Error: [$compile:multidir]
Can you help?
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.
Adding the attribute addressBook
breaks this.
We have a directive which is called addressBook
, so Angular doesn't know if it should now render the addressBook or the propertyGroup directive.
You have to choose a different attribute name.
@@ -8,6 +8,7 @@ angular.module('contactsApp') | |||
name: '=', | |||
data: '=', | |||
contact: '=model', | |||
addressBook: '=addressBook', |
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.
Just FYI this line is to specific: addressBook: '='
would suffice.
You only need to write something after the =
if the attribute should be named differently to the controller variable.
templates/detailItems/adr.html
Outdated
<option ng-repeat="option in ctrl.availableOptions" value="{{option.id}}">{{option.name}}</option> | ||
</select> | ||
<button ng-click="ctrl.deleteField()" class="icon-delete" title="{{ctrl.t.delete}}" ng-if="!ctrl.model.addressBook.readOnly"></button> | ||
<button ng-click="ctrl.deleteField()" class="icon-delete" title="{{ctrl.t.delete}}" ng-if="!ctrl.addressBook.readOnly"></button> |
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 would suggest that we offer a readOnly property for contacts, which reflects the readOnly property of the addressbook.
This way we also don't have to pass the addressbook to the propertyGroup
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
5f0df73
to
53267fc
Compare
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
All clear! @irgendwie @Henni Please review! :) |
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.
Code changes seem reasonable, every field update gets pushed to the server and read-only view is also working again! 👍
Then merge it ;) |
@MorrisJobke Was waiting for another 👍 😉 |
Signed-off-by: John Molakvoæ skjnldsv@protonmail.com