functional refactor & fix: remove state.role #542
Conversation
| return ( | ||
| <Box sx={paneMenuStyle(props.x, props.y)}> | ||
| <MenuList className="paneContextMenu"> | ||
| {!isGuest && ( | ||
| {editing && ( |
There was a problem hiding this comment.
Is it better to control the parent by editing, rather than repeating the editing check on each MenuItem ?
There was a problem hiding this comment.
There could be some context menu items in "view" mode as well.
| edge="end" | ||
| aria-label="delete" | ||
| onClick={() => | ||
| deleteCollaborator({ |
There was a problem hiding this comment.
I don't think non-owner user can delete other collaborators. I know it must give an error message from database update operation because of no auth. But in the front-end, we should still disable it.
There was a problem hiding this comment.
You're right; the current API-auth will throw errors.
I'm trying to remove user/auth-related code from the packages/ui so that it can be better re-used across desktop-app and web-app. Things will break. But I consider this as a non-critical break.
Actually, we likely need to add more refined permission control here, e.g., collaborators should be able to add/remove (the behavior of Google Docs).
This is one of the efforts to make the component more "(purely) functional" by extracting states out. States are global variables that should be avoided when possible.
Removed states:
state.role. Also removed all isGuest checks (replaced by a more functional state.editMode below).state.user. The user is only used in Awareness and is now passed as function parameter toconnectYjsaction.state.repoLoadedAdded states:
state.editMode: "edit" | "view". This could be set on-demand for web-ui when loading user authentication module.Bug fix:
Code.tsxandRich.tsx, the Wrap didn't use correct width/height when the role was in "Guest".