Repository navigation
Removing default name for new contact and leaving it blank - #488
rutujasurve94 wants to merge 7 commits into
Conversation
|
@rutujasurve94 welcome to the Contacts app! 🎉 :) @skjnldsv @Henni @juliushaertl this needs a Javascript review :) |
|
I believe this leads to issues as the full name has to be defined. (Possibly fixed by #490) |
|
Removing these lines is mostly fine, but we still have to catch imported contacts which do not have a FN field. So getting rid of the check for But to achieve the goal of this PR, the contact_model.js has to be modified as well. |
|
@Henni thanks a lot for the review, I'll modify the code soon and update it here |
Codecov Report
@@ Coverage Diff @@
## master #488 +/- ##
==========================================
+ Coverage 15.8% 15.88% +0.07%
==========================================
Files 61 61
Lines 1379 1379
==========================================
+ Hits 218 219 +1
+ Misses 1161 1160 -1
Continue to review full report at Codecov.
|
…/rutujasurve94/contacts into feature/FN-field-empty-behaviour
|
@Henni @jancborchardt I have modified the code in the file contact_model.js to substitute 'New Contact' with an empty string. |
| angular.extend(this.props, { | ||
| version: [{value: '3.0'}], | ||
| fn: [{value: t('contacts', 'New contact')}] | ||
| fn: [{value: t('contacts', '')}] |
There was a problem hiding this comment.
The t() function translates a string to the local language. We don't need to translate ''. So just replace t('contacts', '') with ''.
| newContact.uid(newUid); | ||
| newContact.setUrl(addressBook, newUid); | ||
| newContact.addressBookId = addressBook.displayName; | ||
| if (_.isUndefined(newContact.fullName()) || newContact.fullName() === '') { |
There was a problem hiding this comment.
We still need to check if the fullName of newContact is undefined. So please add the first half of this check back in.
…/rutujasurve94/contacts into feature/FN-field-empty-behaviour
…/rutujasurve94/contacts into feature/FN-field-empty-behaviour
…/rutujasurve94/contacts into feature/FN-field-empty-behaviour
…/rutujasurve94/contacts into feature/FN-field-empty-behaviour
|
@Henni @jancborchardt I have modified the code according to the review |
|
@nextcloud/contacts need further review here :) |
skjnldsv
added
enhancement
skjnldsv
left a comment
skjnldsv
left a comment
There was a problem hiding this comment.
I'll still do't really like the idea of having empty contacts laying around.
- The name field is still not docused on contact creation
- Server still refusing the card
- We need to make sure we don't have any issue when sorting the contact list if there are empty contacts in it
| angular.extend(this.props, { | ||
| version: [{value: '3.0'}], | ||
| fn: [{value: t('contacts', 'New contact')}] | ||
| fn: [{value: ''}] |
There was a problem hiding this comment.
Server is still refusing the vcard
Validation error in vCard: The FN property must appear in the VCARD component exactly 1 time
I'll suggest using a space here
skjnldsv
added
2. developing
5 participants
Removing the check that puts the default name 'New Contact' in case the contact field is new