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

Vue cdav copy addressbook #639

Merged
merged 25 commits into from Sep 25, 2018

Conversation

Projects
2 participants
@sleepypioneer
Member

sleepypioneer commented Sep 25, 2018

Please check my code, not feeling so confident with this

skjnldsv and others added some commits Sep 20, 2018

Dav client service and first checks
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Really load the server contacts
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Removed unwanted config, better error handling and preload contacts
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Delete dav handler
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Fixed deletion on details, improved loading performances and comments
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Contact save and loader
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Add addressbook dav
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Add addressbook dav
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
NC components update
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Regex fix
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Fixed select property
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Multiseelct and comments fix
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Css pulse and warning on unsaved vcards
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
JSDoc fixes
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Contact deletion fix and warning if fetchFull fails and/of contact gone
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Home mobile compatibility addition
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

@sleepypioneer sleepypioneer added this to the 3.0.0 milestone Sep 25, 2018

@sleepypioneer sleepypioneer self-assigned this Sep 25, 2018

@sleepypioneer sleepypioneer added this to 👩‍💻 In progress in Rails Girls Summer of Code 2018 via automation Sep 25, 2018

@sleepypioneer sleepypioneer requested a review from skjnldsv Sep 25, 2018

@skjnldsv

skjnldsv requested changes Sep 25, 2018 edited

Works nicely! 😀
Just a small change in comments.

But I'm wondering if we should not manually open the link to a new tab, as a feedback, and as the error string have something like: 'Could not copy, opening instead'
What do you think?

this.copySuccess = true
this.copied = true
// timeout sets the text back to copy to show text was copied
setTimeout(() => {

This comment has been minimized.

@skjnldsv

skjnldsv Sep 25, 2018

Member

Hum,timeout are bad feedback when they're not relevant. Here the string is copied right away but we get the feeling we need to wait :)

This comment has been minimized.

@sleepypioneer

sleepypioneer Sep 25, 2018

Member

Ok I just thought otherwise it really doesn't show as its pretty quick!

@sleepypioneer

This comment has been minimized.

Member

sleepypioneer commented Sep 25, 2018

Hi @skjnldsv I removed the time out and added notifications. However I then noticed that the download addressbook I did the same trick to give the indication something was happening so should I remove that also??

@skjnldsv

This comment has been minimized.

Member

skjnldsv commented Sep 25, 2018

@sleepypioneer I guess so yes :)
I did not noticed before, sorry 🙈

@sleepypioneer

This comment has been minimized.

Member

sleepypioneer commented Sep 25, 2018

done, I also added notifications for copy and delete for if they were successful, otherwise I think it is unclear something happened.

this.downloadLoading = false
}, 1500)
// Notify download started
OC.Notification.showTemporary(t('contacts', 'Downloading Addressbook'))

This comment has been minimized.

@skjnldsv

skjnldsv Sep 25, 2018

Member

Not needed, if the download starts, the user already knows it ;)

This comment has been minimized.

@sleepypioneer

sleepypioneer Sep 25, 2018

Member

ok will remove it :) anything else before merging?

This comment has been minimized.

@skjnldsv

skjnldsv Sep 25, 2018

Member

🙈 😬 🚀 GO

@sleepypioneer sleepypioneer merged commit 9a7d7b8 into vue-cdav-lib Sep 25, 2018

Rails Girls Summer of Code 2018 automation moved this from 👩‍💻 In progress to 🎉 Done! Sep 25, 2018

@sleepypioneer sleepypioneer deleted the vue-cdav-copy-addressbook branch Sep 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment