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

Added possibility to show and edit vCard Geo attributes #1250

Merged
merged 1 commit into from
Sep 19, 2019

Conversation

tacruc
Copy link
Contributor

@tacruc tacruc commented Sep 5, 2019

Signed-off-by: Arne Hamann kontakt+github@arne.email

@tacruc tacruc requested a review from skjnldsv September 5, 2019 20:53
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! :)
We could standardise a bit more! See comments! 🚀 👍

@skjnldsv skjnldsv added 3. to review Waiting for reviews enhancement New feature or request labels Sep 6, 2019
@skjnldsv skjnldsv added this to the next milestone Sep 6, 2019
@skjnldsv skjnldsv mentioned this pull request Sep 6, 2019
@skjnldsv skjnldsv self-requested a review September 6, 2019 12:57
src/models/rfcProps.js Outdated Show resolved Hide resolved
@skjnldsv
Copy link
Member

skjnldsv commented Sep 9, 2019

@tacruc so what do you think, good enough?
Is it working properly on your side and fitting your needs? :)

Signed-off-by: Arne Hamann <kontakt+github@arne.email>
@codecov
Copy link

codecov bot commented Sep 19, 2019

Codecov Report

Merging #1250 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1250   +/-   ##
======================================
  Coverage      60%     60%           
======================================
  Files           4       4           
  Lines          60      60           
======================================
  Hits           36      36           
  Misses         24      24

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a953307...0b3bbdd. Read the comment docs.

@skjnldsv skjnldsv merged commit c04d1b9 into master Sep 19, 2019
@welcome
Copy link

welcome bot commented Sep 19, 2019

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/contacts/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
Most developers hang out on IRC. So join #nextcloud-contacts and #nextcloud-dev on Freenode for a chat!

@skjnldsv skjnldsv deleted the geo-attibute branch September 19, 2019 21:25
@skjnldsv skjnldsv modified the milestones: next, 3.1.4 Oct 3, 2019
@pieter-groeneweg
Copy link

pieter-groeneweg commented Oct 17, 2019

I see this is under review (label "3. to review" and "enhancement") for milestone 3.1.4...
3.1.4 is completed as is 3.1.6 which I have installed.
Out of curiousity... when can we expect GEO attributes in the contacts?

Noticed it is called "location" super. However it asks for a URL?

Checking a vcf export I see this:

BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.1.6//EN
UID:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
REV;VALUE=DATE-AND-OR-TIME:20191021T115002Z
FN:Name
ADR;TYPE=HOME:;;Street 1;City;;PostCode;Country
EMAIL;TYPE=HOME:email@something.com
TEL;TYPE="HOME,VOICE":+00 000 000000
CATEGORIES:category
GEO;VALUE=URI:geo:5.0000\,52.0000
END:VCARD

If I read this: https://en.wikipedia.org/wiki/VCard
Since the export is V3
the GEO should be (v3) GEO:5.00000,52.00000

If the export was V4
the GEO should be GEO:geo:5.00000,52.00000

the parameter notation: VALUE=URI:geo:5.0000,52.0000 is to be used in other properties such as additional BIRTHPLACE or DEATHPLACE...

Also the separating comma should not be escaped (preceded by ) I believe...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants