-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(web): add remove member button #841 #888
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for the contribution @MaurerKrisztian I've attached a design from the Figma project in the comment. Could you please take a look at it?
async function removeMemberClick(member) { | ||
await removeMember(member._id) | ||
.then((result) => { | ||
showNotification({ | ||
message: `Successful member deletion.`, | ||
color: 'green', | ||
}); | ||
refetch(); | ||
|
||
return result; | ||
}) | ||
.catch((err) => { | ||
showNotification({ | ||
message: err.message, | ||
color: 'red', | ||
}); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async function removeMemberClick(member) { | |
await removeMember(member._id) | |
.then((result) => { | |
showNotification({ | |
message: `Successful member deletion.`, | |
color: 'green', | |
}); | |
refetch(); | |
return result; | |
}) | |
.catch((err) => { | |
showNotification({ | |
message: err.message, | |
color: 'red', | |
}); | |
}); | |
} | |
async function removeMemberClick(member) { | |
try { | |
await removeMember(member._id) | |
showNotification({ | |
message: `Successful member deletion.`, | |
color: 'green', | |
}); | |
refetch(); | |
} catch (e) { | |
showNotification({ | |
message: err.message, | |
color: 'red', | |
}); | |
} | |
} |
A small suggestion can be refactored to use plain async await like the rest of the codebase. Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree.
<span onClick={() => removeMemberClick(member)}> | ||
<Tag css={{ cursor: 'pointer' }}>X</Tag> | ||
</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please checkout the design at figma:
https://www.figma.com/file/IpAw7sGWYZ8DHTem0tSM8r/Ready-for-Dev?node-id=28:7377
So we can actually have the 3 dots with some space around, and upon clicking we can use mantine's dropdown with a delete button. You can also see Workflow editor for example.
Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export function ThreeDot({ ...props }) { | ||
const { theme } = useStyles(); | ||
|
||
const dot = { | ||
height: '7px', | ||
width: '7px', | ||
margin: '2px', | ||
'background-color': theme.colorScheme == 'dark' ? '#525266' : '#525266', | ||
'border-radius': '50%', | ||
display: 'inline-block', | ||
'font-weight': 'bold', | ||
}; | ||
|
||
const threeDot = { | ||
cursor: 'pointer', | ||
padding: '5px', | ||
margin: '5px', | ||
}; | ||
|
||
return ( | ||
<div style={threeDot}> | ||
<span style={dot}></span> | ||
<span style={dot}></span> | ||
<span style={dot}></span> | ||
</div> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for missed this one, but we actually just have an SVG icon for the three dots so no need to recreate it. It located in apps/web/src/design-system/icons/general/DotsHorizontal.tsx
So you can just import it instead of building it with CSS.
🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, sorry I didn't noticed
This now looks awesome @MaurerKrisztian, been playing with it right now. A few things noticed:
Wdyt? |
@scopsy Good idea. I added that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @MaurerKrisztian for your contribution, I've added some unit tests for this aswell with the latest cypress component update. Merging this!
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature
What is the current behavior? (You can also link to an open issue here)
Team Members management #841
What is the new behavior (if this is a feature change)?
Admin can remove invitation or members (if backend enables)
Other information: