h3n3/cadhub
1
0
Fork 0
Code Issues 35 Pull Requests Packages Projects Releases Wiki Activity

Add project forking #533

Merged
franknoirot merged 7 commits from frank/add-project-forking into main 2021-09-28 14:51:36 +02:00
Conversation 7 Commits 7 Files Changed 17 +351 -252
franknoirot commented 2021-09-27 14:32:09 +02:00 (Migrated from github.com)
Copy Link

Adds the ability to fork projects from either the ProjectProfile or Editor routes if user is logged-in.

This design frame shows both the Fork button in the header navbar and the link to a project this was forked from within the details panel. Here is that view in local development:
Screen Shot 2021-09-27 at 8 29 01 AM

This design shows the Forks counter in the ProjectCard component. Here is that count shown in development (disregard the missing images, never took a project image for them):
Screen Shot 2021-09-27 at 8 30 59 AM

Adds the ability to fork projects from either the ProjectProfile or Editor routes if user is logged-in. [This design frame](https://www.figma.com/file/VUh53RdncjZ7NuFYj0RGB9/CadHub?node-id=1057%3A2294) shows both the Fork button in the header navbar and the link to a project this was forked from within the details panel. Here is that view in local development: <img width="1440" alt="Screen Shot 2021-09-27 at 8 29 01 AM" src="https://user-images.githubusercontent.com/23481541/134908103-c96c82ac-2dca-4e1a-98bd-35c215e7e2e6.png"> [This design](https://www.figma.com/file/VUh53RdncjZ7NuFYj0RGB9/CadHub?node-id=1150%3A1619) shows the Forks counter in the `ProjectCard` component. Here is that count shown in development (disregard the missing images, never took a project image for them): <img width="909" alt="Screen Shot 2021-09-27 at 8 30 59 AM" src="https://user-images.githubusercontent.com/23481541/134908359-2b01eebe-a07e-46e0-b8ec-db63af1a5efa.png">
🚀 1
franknoirot (Migrated from github.com) reviewed 2021-09-28 05:25:34 +02:00
app/web/src/components/IdeHeader/IdeHeader.tsx
@@ -144,2 +141,2 @@
name="Save Project Image"
className=" bg-ch-blue-650 bg-opacity-30 hover:bg-opacity-80 text-ch-gray-300"
className="bg-ch-pink-800 bg-opacity-30 hover:bg-opacity-80 text-ch-gray-300"
onClick={handleRender}
franknoirot (Migrated from github.com) commented 2021-09-28 00:23:45 +02:00
Copy Link

Navigate to either the IDE or the details views to match wherever you are now.

Navigate to either the IDE or the details views to match wherever you are now.
franknoirot (Migrated from github.com) commented 2021-09-28 00:25:37 +02:00
Copy Link

TODO regularize these params when the Header component gets regularized.

TODO regularize these params when the Header component gets regularized.
app/web/src/index.css
@@ -52,6 +52,7 @@
.markdown-overrides div {
@apply text-ch-gray-300 bg-transparent;
}
.pink-link,
franknoirot (Migrated from github.com) commented 2021-09-28 00:29:12 +02:00
Copy Link

Made this a utility class and naming things is hard.

Made this a utility class and naming things is hard.
Irev-Dev (Migrated from github.com) reviewed 2021-09-28 12:32:39 +02:00
app/web/src/components/IdeHeader/IdeHeader.tsx
@@ -144,2 +141,2 @@
name="Save Project Image"
className=" bg-ch-blue-650 bg-opacity-30 hover:bg-opacity-80 text-ch-gray-300"
className="bg-ch-pink-800 bg-opacity-30 hover:bg-opacity-80 text-ch-gray-300"
onClick={handleRender}
Irev-Dev (Migrated from github.com) commented 2021-09-28 12:32:39 +02:00
Copy Link

This seems a little brittle to me, as I would hope that the component would be able be able to know what page it's on without having to look at the route. What if someone names their project "idea" than it would match on "user/idea" I had a look at seeing what it would take to pass this information to the component instead and realised that it could be done but it would be adding more mess to the messy situation that is this component so I did a big refactor aimed at this branch.

I've put you as a review, but the diff is not super meaningful as it's really just huge chunks of code moved around 🤷

https://github.com/Irev-Dev/cadhub/pull/534

This seems a little brittle to me, as I would hope that the component would be able be able to know what page it's on without having to look at the route. What if someone names their project "idea" than it would match on "user/idea" I had a look at seeing what it would take to pass this information to the component instead and realised that it could be done but it would be adding more mess to the messy situation that is this component so I did a big refactor aimed at this branch. I've put you as a review, but the diff is not super meaningful as it's really just huge chunks of code moved around 🤷 https://github.com/Irev-Dev/cadhub/pull/534
Irev-Dev (Migrated from github.com) reviewed 2021-09-28 12:41:26 +02:00
app/web/src/components/ProjectCell/ProjectCell.tsx
Irev-Dev (Migrated from github.com) commented 2021-09-28 12:41:26 +02:00
Copy Link

I can't remember if the formatting for the forks always looked like this in the designs or not
image
So I'll ask, are you querying for the userName because thats how you want to fork formatted,

I can't remember if the formatting for the forks always looked like this in the designs or not ![image](https://user-images.githubusercontent.com/29681384/135072198-362c13f1-0ae5-4337-9c6f-e44b3e5dfeea.png) So I'll ask, are you querying for the userName because thats how you want to fork formatted, <title> by <user> or because you need the username to link to a project?
Irev-Dev (Migrated from github.com) reviewed 2021-09-28 12:44:58 +02:00
app/web/src/components/ProjectCell/ProjectCell.tsx
Irev-Dev (Migrated from github.com) commented 2021-09-28 12:44:58 +02:00
Copy Link

Add id, can cause caching issues with Apollo

          user {
            id
Add id, can cause caching issues with Apollo ```suggestion user { id ```
Irev-Dev (Migrated from github.com) approved these changes 2021-09-28 12:52:10 +02:00
Irev-Dev (Migrated from github.com) left a comment
Copy Link

Looks great.

Listing some things that have some to mind that we'll also want to consider going forward.

  • A way of viewing childForks of a project
  • Add a fork relationship to the db seed (the db seed is very light atm could do with bulking up in general)
  • Adding your badge when people start editing code from someone else's project
    image
Looks great. Listing some things that have some to mind that we'll also want to consider going forward. - A way of viewing childForks of a project - Add a fork relationship to the db seed (the db seed is very light atm could do with bulking up in general) - Adding your badge when people start editing code from someone else's project ![image](https://user-images.githubusercontent.com/29681384/135073615-26423b68-163c-4718-9316-f6b5c48e4169.png)
Irev-Dev commented 2021-09-28 12:52:49 +02:00 (Migrated from github.com)
Copy Link

Up to you if you want to merge this or https://github.com/Irev-Dev/cadhub/pull/534/files first.

Up to you if you want to merge this or https://github.com/Irev-Dev/cadhub/pull/534/files first.
Irev-Dev commented 2021-09-29 10:16:43 +02:00 (Migrated from github.com)
Copy Link

Live now, sorry to me a while as I broke the build for pre-rendering in #531

Live now, sorry to me a while as I broke the build for pre-rendering in #531
Sign in to join this conversation.
Reviewers
No Reviewers
franknoirot
Irev-Dev
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
h3n3
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: h3n3/cadhub#533
Delete Branch "frank/add-project-forking"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?