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

Phonetic name support #251

Closed
nickvergessen opened this issue Jun 30, 2017 · 23 comments · Fixed by #1741
Closed

Phonetic name support #251

nickvergessen opened this issue Jun 30, 2017 · 23 comments · Fixed by #1741
Labels
1. to develop Accepted and waiting to be taken care of enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@nickvergessen
Copy link
Member

nickvergessen commented Jun 30, 2017

Should we downstream https://github.com/owncloud/contacts/pull/595/files ?

Add new props (see #251 (comment)) into the rfcProps model!


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@nickvergessen nickvergessen added enhancement New feature or request feature parity Feature on other projects that needs to be implemented labels Jun 30, 2017
@MariusBluem
Copy link
Member

I see no reason why not 😅

@jancborchardt
Copy link
Member

Yes, everything should be downstreamed. Also cc @nextcloud/contacts and the original patch submitter @t-bucchi :)

@norbusan
Copy link

Yes yes yes!!! Please. Living in Japan I am in dire need of phonetic name support. Thanks!

@t-bucchi
Copy link
Contributor

Hi, I'm a original patch submitter.
I want to merge this patch to nextcloud also.
Is there anything I can do for you?
Thanks.

@jancborchardt
Copy link
Member

@t-bucchi cool, welcome! :) You could either cherry-pick the commits with gits functionality, or manually apply the changes since maybe the code underneath changed meanwhile.

If you have any questions, @nextcloud/contacts can help you. :) You are also welcome in our chat channel at #nextcloud-contacts! 🎉

@skjnldsv
Copy link
Member

skjnldsv commented Aug 13, 2018

@jancborchardt it could be great to add it into the 3.0 update, but the code is completely different.

I'd like to know a few informations @t-bucchi
X-PHONETIC-FIRST-NAME is not an official prop, so before we go for it, I'd like to ve sure this is supported on other systems. Google is ok with this? What does it do on Android? iOS?

Davdroid seems to use this: https://www.davdroid.com/manual/supported-contact-fields/
Which I'm ok with. But creating invalid vcards by extending the N field to more props, I'm not really ok with that
https://developer.android.com/reference/android/provider/ContactsContract.CommonDataKinds.StructuredName

@t-bucchi
Copy link
Contributor

@jancborchardt Thanks for your reply. I will merge my patch to current master.

@skjnldsv Google Contacts on web, Android and iOS support X-PHONEIC-FIRST-NAME. I attached vCards are exported from iOS and Google Contacts. Could you confirm its?
google.vcf.txt
ios.vcf.txt

And, my patch doesn't extend N field. It changes name order when generates full name from N field to display. Because, in Japan, we use "LAST-NAME FIRST-NAME" order generally.

@skjnldsv
Copy link
Member

@t-bucchi thanks for the feedback!
Android extend the n property, that's why I mentioned it :)

I'm ok with using the X-PHONETIC props. It looks like the proper way to do since the rfc allow us to do so. Let's try to add this in 3.0.0

@t-bucchi
Copy link
Contributor

@skjnldsv Thanks! I'm going to merge.

@skjnldsv
Copy link
Member

@t-bucchi to merge?

@t-bucchi
Copy link
Contributor

@skjnldsv Ah, I mean rebase or adjust my patch to head of master branch. I don't need to do it?
I'm creating the environment to develop. Current nextcloud/contacts looks working on Nextcloud 14 beta only.
What the revision of nextcloud/server should I use? Is head OK?

@skjnldsv
Copy link
Member

We'll implement it in the 3.0 update. Your patch is not compatible. Don't worry we'll take care of it! 🤗

@t-bucchi
Copy link
Contributor

@skjnldsv Oh! Thanks! I'm looking forward to 3.0 update!

@skjnldsv skjnldsv added this to the 3.0.1 milestone Sep 28, 2018
@skjnldsv skjnldsv added 1. to develop Accepted and waiting to be taken care of good first issue Good for newcomers Hacktoberfest labels Oct 2, 2018
@skjnldsv skjnldsv removed feature parity Feature on other projects that needs to be implemented labels Oct 28, 2018
@skjnldsv skjnldsv modified the milestones: 3.0.1, 3.1.0 Dec 24, 2018
@norbusan
Copy link

Is there any progress wrt to phonetic name support? There has been a patch, lots of updates, and at the end we are at NC16 and still without phonetic name support.

Could one of the devs please explain what the problem is? Thanks

@skjnldsv

This comment has been minimized.

@norbusan

This comment has been minimized.

@skjnldsv

This comment has been minimized.

@skjnldsv
Copy link
Member

skjnldsv commented Jun 13, 2019

I may be overreacting.
Sorry, I think I was hoping for a more encouraging message and yours depressed me a bit seeing that there was indeed a lot of time passed and still some nice pending features. 😖

So please accept my apologies. Let's be productive here! 🤗


This requires a bit of work as you could simply add two independent input in the rfcprops.js file, but this is quite not the best ux/ui. Ideally we should add two more inputs into the N field, but the way our data structure is designed does not really allow a split between a single property (vue) and multiple properties (vcard).

@norbusan
Copy link

Hi @skjnldsv
thanks for your remarks about how to tackle the problem. I will take a look. I know my way around in angular, but you have switched to vue, which I don't know at all.

Is there at least a general agreement on the vcard field naming? Reading the thread here I don't get the impression that going with X-PHONETIC-FIRST-NAME etc is accepted.

Thanks

Norbert

PS: nothing to worry about your comment, and I agree I could have been less pushy, sorry for that.

@skjnldsv
Copy link
Member

skjnldsv commented Jun 14, 2019

Is there at least a general agreement on the vcard field naming? Reading the thread here I don't get the impression that going with X-PHONETIC-FIRST-NAME etc is accepted.

Any X- property is valid according to the rfc :)
It is not officially supported, but other big orgs like to create vcards standards (a bit like css webkit/moz prefixes before the official css releases)
So Google is using a a dedicated property, but Davdroid created a consensus and matches the two automatically https://www.davx5.com/manual/technical_information.html#name

Thanks!! 🤗

@t-bucchi
Copy link
Contributor

t-bucchi commented Aug 9, 2020

Hi. I wrote the new patch for this issue based on v3.3.0 .
I want to send pull request.
But, the master looks changing rapidly (many marge).
So, it's difficult to test my patch based on master.
Should I wait until the merge is complete?

@skjnldsv
Copy link
Member

skjnldsv commented Aug 9, 2020

@t-bucchi open a pr, I'll help you get it merged even with master changes, don't worry :)
The two other big prs are #1687 and #1688, and it's still in need for some work, so we can try to merge yours before the two others ;)

@t-bucchi
Copy link
Contributor

t-bucchi commented Aug 9, 2020

@skjnldsv Thank you for your help!
I'll send pull request soon.
Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants