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

Feat: add a top-right share-button on the repo page #152

Merged
merged 3 commits into from
Dec 16, 2022

Conversation

li-xin-yi
Copy link
Collaborator

Change:

  1. Remove the share-buttons on the dashboard, add it on the repo page.

image

  1. Polish the share-project dialog, everyone accessing the repo can click the share-button (and the share option in right-click menu) to see the dialog, but the privilege levels vary:

image

  • A read-only URL field to copy
  • An owner can switch the visibility (public or private) by a button (ref:feat: create repo without a name; generate userId and repoId from server #142 (comment))
  • Explanation about the visibility when clicking the help icon
  • Fetch a list of current collaborators, the owner can delete any of them by clicking the button
  • Invite others by collaborators (if you are not the owner, it doesn't work)
  • Change of the visibility (isPublic and the list of collaborators) loads immediately after actions.

By the way, if there are no collaborators now, it looks like:

image

Please test carefully before merging.

Related to-do list:

  1. Code involving error displaying looks quite messy, consider restructuring it via useReducer if I have time.
  2. Recover the share-button on the dashboard if possible. I remove it mainly because the timer for TTL results in re-rendering dialog every second and hard to write those remote queries without store. But it is very inconvenient to share a project by entering it first.
  3. Consider completely hiding/disabling the invitation context for non-owner user in the future. I just remain it and throw the access error when submitting for now.

Copy link
Collaborator

@lihebi lihebi left a comment

Choose a reason for hiding this comment

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

Thanks, Xinyi! This feature runs very well. Overall the code looks great to me. I left a couple of comments.

ui/package.json Show resolved Hide resolved
Comment on lines 71 to 76
<MenuItem onClick={props.onShareClick} sx={ItemStyle}>
<ListItemIcon>
<ShareOutlinedIcon />
</ListItemIcon>
<ListItemText> Share </ListItemText>
</MenuItem>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could remove this right-click menu item now.

ui/yarn.lock Show resolved Hide resolved
Comment on lines 43 to 44
inRepo = false,
setShareOpen = () => {},
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. It seems that setShareOpen doesn't need to be passed in. You could call useStore to get it.
  2. inRepo isn't general enough. I would use a buttonItem=null argument and pass in a ReactNode (just like the breadcrumbItem above). This way, the repo sharing UI & logic are not leaked into Header's implementation.

E.g.,

// repo.tsx
function ShareButton() {
  const setShareOpen = useStore(...);
  return <Box><Button onClick={()=>setShareOpen(true)}>...</Button></Box>
}
function RepoWrapper() {
  return <Box> ... <Header buttonItem={<ShareButton/>}/></Box>
}

// Header.tsx
function Header({..., buttonItem=null}) {
  return <Box> ... {buttonItem} ... </Box>
}

@li-xin-yi
Copy link
Collaborator Author

li-xin-yi commented Dec 16, 2022

Update:

  1. remove share-option in the right-click menu
  2. remove inRepo, pass the button component to shareButton prop
  3. disable the invite-button and email input filed for non-owner users in share-dialog.
  4. compress the parameter list
  5. use useReducer to manage various feedback messages in share-dialog to make the code more readable.

Copy link
Collaborator

@lihebi lihebi left a comment

Choose a reason for hiding this comment

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

This is a great feature and code, thanks!

@lihebi lihebi merged commit a03da22 into codepod-io:main Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants