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

Private Sketch feat. #3034

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

vivekbopaliya
Copy link
Contributor

@vivekbopaliya vivekbopaliya commented Feb 18, 2024

Changes:

bandicam.2024-05-04.13-33-34-726.mp4

WORK FLOW:

  • Every sketch is default public, user can change to Private.
  • Private sketch will be visible to owner of project only.
  • If someone who is not owner tries to access Private project through URL, they will be redirected to /
  • Filtering is done on backend.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste
Copy link
Collaborator

This is awesome. I can tell that you have put a lot of thought into it and done some very good work here! I'm sure @raclim will have some thoughts on the UI. I'm going to see if I can figure out why you're getting errors in the getProjectsForUser handler. I'm not sure what steps we need to take as far as backwards compatibility, as existing projects won't have a visibility property. Possibly it's enough just to set default: 'Public' on the Schema?

@vivekbopaliya
Copy link
Contributor Author

I'm not sure what steps we need to take as far as backwards compatibility, as existing projects won't have a visibility property. Possibly it's enough just to set default: 'Public' on the Schema?

Yeah, that will work. will do that. However, I have set up the state like this in Toolbar.jsx:

const [visibility, setVisibility] = React.useState(
    project.visibility || 'Public'
);

So for old sketches, the default value "Undefined" will be replaced by "Public" in the state. This way, the user should be able to change the visibility of old sketches as well.

@vivekbopaliya
Copy link
Contributor Author

@lindapaiste @raclim still waiting for UI to be approved so that i can go ahead

@raclim
Copy link
Collaborator

raclim commented Mar 18, 2024

Thanks so much for working on this and I'm sorry for the delay in getting to this!

Honestly, this looks really awesome and I don't think I have much else to add so far on the UI! I know there were some previous designs done for a potential Privacy feature. It's a bit late, but you can probably still choose to incorporate them if you'd like, but I think what's currently here is great as well.

I think one potential next step for this after some further development is having it tested out with a small group of users who can probably give further feedback from there on the design/functionality as well. I can also try to start tagging and sharing it with some folks if it makes more sense to do it sooner rather than later down the line!

@vivekbopaliya
Copy link
Contributor Author

I know there were some previous designs done for a potential Privacy feature.

This whole look is quite so cool and refreshing actually. I'm excited about adding it to my design. It's got a premium vibe that I think will work really well. However, it seems like it will require a significant amount of effort on its own. I'm currently tied up with some other tasks for the time being, but I'll try to dedicate some time to it within the next month and raise a PR. In the meantime, It would be best to focus on its functionality and gathering feedback related to it. it's better to move forward by sharing it with some individuals to gather feedback on its functionality.

@vivekbopaliya
Copy link
Contributor Author

vivekbopaliya commented May 4, 2024

It's a bit late, but you can probably still choose to incorporate them if you'd like, but I think what's currently here is great as well.

@raclim Just a quick update: I've created a new PR with changes inspired by the design. let me know your thoughts!

@release-com release-com bot deployed to p5.js-web-editor: privating-sketches (privating-sketches) June 13, 2024 20:30 Active
@raclim
Copy link
Collaborator

raclim commented Jun 13, 2024

@vivekbopaliya Sorry that it took a while, just want to update that I created a shareable environment for this PR! Please feel free to share this around and get some feedback on it!

I feel like it looks pretty solid so far though, and super excited to see this out soon!

Copy link

release-com bot commented Jun 14, 2024

Release Environments

This Environment is provided by Release, learn more!
To see the status of the Environment click on Environment Status below.

🔧Environment Status : https://app.release.com/public/Processing%20Foundation/env-6ee1c7dda9

  • p5.js-web-editor

🔧Environment Status : https://app.release.com/public/Processing%20Foundation/env-adee4fd928

@vivekbopaliya
Copy link
Contributor Author

@vivekbopaliya Sorry that it took a while, just want to update that I created a shareable environment for this PR! Please feel free to share this around and get some feedback on it!

I feel like it looks pretty solid so far though, and super excited to see this out soon!

Finally, it is here! Thank you very much. Let's see how this works out. If it does, we might want to design an implementation for mobile view as well.

@raclim
Copy link
Collaborator

raclim commented Jun 14, 2024

Sounds good, I'll also do some further testing on my end as well!

@Felixpaulsen
Copy link

Nice work! I hope we'll see this public soon as it will also fix issues with private api keys being publicly exposed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants