-
Notifications
You must be signed in to change notification settings - Fork 1
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
Share mass contacts #22
Conversation
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.
Great work ! :-) Just a few improvements below.
} else { | ||
setViewChecked(false); | ||
setShareChecked(false); | ||
removeInvitation(record.describes); | ||
updateRecord({ ...record, canViewEvent: false }) |
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.
If we remove the view right, then we should also remove the share right. (That's the current behaviour)
return ( | ||
<List dense className={classes.list}> | ||
{ids.map((id, i) => ( | ||
{isOrganizer && ( |
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.
It would be important that the "All contacts" share button is also available for non-organizers (in this case, we would see only one toggle)
<ListItemAvatar className={classes.avatarItem}> | ||
<Avatar className={classes.avatar}> | ||
C | ||
</Avatar> |
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 use a Material-UI icon to make this line very different from others ? I suggest this one:
import PublicIcon from '@material-ui/icons/Public';
/> | ||
<ListItemText | ||
className={classes.secondaryText} | ||
primary={translate('app.share.allow_view')} |
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.
IMO it would be better to use the same wording as other lines. There are two existing keys: app.permission.view
and app.permission.share
.
@andreipopa-who Do you think you can finish this PR soon ? I need it for #27. Otherwise I will finish it myself. |
e09ae6e
to
571db1d
Compare
Closing in favor of #38, which takes up on the work done in this PR. Thanks @andreipopa-who ! |
Add a new list item in order to allow the mass selection of view/share on contacts.