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

Feature: Remove Collaboration #488

Merged
merged 10 commits into from
Aug 28, 2020

Conversation

prabalsingh24
Copy link
Contributor

Problem

Collaborators should have an option to remove collaboration from a collection

Solution

Screenshot_2020-08-10 BookBrainz – The Open Book Database(4)

@coveralls
Copy link

coveralls commented Aug 10, 2020

Coverage Status

Coverage increased (+0.06%) to 59.681% when pulling b6b232a on prabalsingh24:remove-collaborator into c6cc0f6 on bookbrainz:UserCollection.

Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Another useful feature !

I wonder if instead of the button being below the list of entries, which might suggest that it's an action on the list items, we could show a really small button or just an icon next the the collaborator's name (next to my own name only), something like this:

Capture d’écran 2020-08-11 à 14 47 08

What do you think?

src/client/components/pages/collection.js Outdated Show resolved Hide resolved
You’re about to remove yourself as a collaborator of Collection: {collection.name}.
</h4>
<p>
Are you sure you want to do this ? You won’t be able to undo this.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Are you sure you want to do this ? You won’t be able to undo this.
Are you sure you want to proceed ? You won’t be able to undo this.

src/server/routes/collection.js Outdated Show resolved Hide resolved
Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

A few more small corrections, some of which I'm just seeing now.

src/server/routes/search.js Outdated Show resolved Hide resolved
src/server/routes/collection.js Outdated Show resolved Hide resolved
src/client/components/pages/collection.js Outdated Show resolved Hide resolved
src/client/components/pages/collection.js Outdated Show resolved Hide resolved
@prabalsingh24
Copy link
Contributor Author

Oops. I made so many silly mistakes in this PR.

@MonkeyDo
Copy link
Contributor

Oops. I made so many silly mistakes in this PR.

Everyone has tired days :)
I know I do !

Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

We're down to nitpicks, this is working perfectly!

Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Great, ready to merge !

@MonkeyDo MonkeyDo merged commit 662215a into metabrainz:UserCollection Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants