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

New Contributors Page for Twenty Website #3365

Closed
Bonapara opened this issue Jan 11, 2024 · 22 comments · Fixed by #3745
Closed

New Contributors Page for Twenty Website #3365

Bonapara opened this issue Jan 11, 2024 · 22 comments · Fixed by #3745
Assignees
Labels
for experienced contributor good first issue Good for newcomers scope: front Issues that are affecting the frontend side only size: short type: marketing

Comments

@Bonapara
Copy link
Member

Bonapara commented Jan 11, 2024

Scope

We aim to highlight our contributors on the Twenty website. We'll develop a webpage featuring each contributor's picture and a dedicated page showcasing their work, including latest pull requests, activity, and other key information.

Desired behavior

Index

CleanShot 2024-01-11 at 09 53 42

Ensure to add a box shadow on image hover: (and use cursor:pointer;)

Screen.Recording.2024-01-11.at.09.58.37.mov

Implement Desktop and Mobile modes. Adjust font size and padding for Mobile mode:

CleanShot 2024-01-11 at 10 00 08

https://www.figma.com/file/aNpCjwN9wg2DqLWAHPS0ll/%F0%9F%8C%8D-Website?type=design&node-id=1-4175&mode=design&t=DOFln1l9kC95T2pW-11

Show page

CleanShot 2024-01-11 at 10 01 18

Implement both desktop and mobile versions of the show page. Link the Github icon to the contributor's profile. Calculate rank based on merged PR count. Replicate the activity graph function from Github.

Navbar

CleanShot 2024-01-11 at 10 07 07

Create a navbar component for both desktop and mobile, mirroring Twenty.com's Framer-developed version. Ensure a transition exists between open and closed states on mobile:

Screen.Recording.2024-01-11.at.10.13.58.mov

Background Pattern

Pattern:

(Compressed to maintain file quality)

Patterns.zip

Website pattern

Here is the code used on Framer to implement the background pattern:

The typical parameters were:
Rotation: -16°
Gradient Rotation: 185°

import * as React from "react"
import { addPropertyControls, ControlType } from "framer"

type Props = {
    image: string
    rotation: number
    gradientRotation: number
}

export function patternBackground(props: Props) {
    // Generate the CSS for the background image
    const backgroundImageStyle = {
        position: "absolute",
        top: "-50%",
        left: "-50%",
        width: "200%", // Increased to cover the area when rotated
        height: "200%", // Increased to cover the area when rotated
        backgroundImage: `url(${props.image})`,
        backgroundSize: "auto 20px", // Adjust for maintaining aspect ratio
        backgroundRepeat: "repeat",
        transform: `rotate(${props.rotation}deg)`,
        transformOrigin: "center center",
    }

    // Generate the CSS for the gradient overlay
    const gradientOverlayStyle = {
        position: "absolute",
        top: 0,
        left: 0,
        right: 0,
        bottom: 0,
        background: `linear-gradient(${props.gradientRotation}deg, #FFF 8.33%, rgba(255, 255, 255, 0.08) 48.95%, #FFF 92.18%)`,
        zIndex: 1, // On top of the background image
    }

    // Generate the CSS for the container
    const containerStyle = {
        height: "100%",
        width: "100%",
        position: "relative",
        overflow: "hidden",
    }

    // Return a div with the container, background image, and gradient overlay
    return (
        <div style={containerStyle}>
            <div style={backgroundImageStyle} />
            <div style={gradientOverlayStyle} />
        </div>
    )
}
})

Technical input

Some work has already be done on the subject. Check:

https://github.com/twentyhq/twenty/tree/main/packages/twenty-website/src/app/developers/contributors

And the API:

packages/twenty-website/src/app/developers/contributors/api/generate/route.tsx

Figma

https://www.figma.com/file/aNpCjwN9wg2DqLWAHPS0ll/%F0%9F%8C%8D-Website?type=design&node-id=5-4946&mode=design&t=DOFln1l9kC95T2pW-11

@Kanav-Arora I hope you don't mind that I used your username as an example ;)

@Kanav-Arora
Copy link
Contributor

@Bonapara ofcourse not

@FelixMalfait
Copy link
Member

Note part of the work for this has already been done (pages already exist) so this is mostly about frontend / UI.
The way it works is you have to hit the /generate endpoint first to fill the SQLite database with Github data.

@Kanav-Arora
Copy link
Contributor

@Bonapara @FelixMalfait
There can be a section which fetches the latest 3 PRs, which can be done by Github API and clicking on it will take you to that PR. This will also help move traffic to Twenty's Github Repo

@FelixMalfait
Copy link
Member

@Kanav-Arora the backend logic is mostly there already. We do a full copy of Github's data to a SQLite database in Next.js. We display every PR that has been done by that user on each user profile.
For the homepage we only display the list of contributors for now but later we'll improve and surface good first issues, organize challenges with rewards, etc
We also plan to use Danger.js to reply to contributions and show user we've created a profile for them / recognize when they reach new steps.

@i-am-chitti
Copy link
Contributor

@FelixMalfait, @Bonapara Can I work on this? Since backend is ready, I'm up for developing the frontend.

@Kanav-Arora
Copy link
Contributor

Hi @i-am-chitti
Since this issue has a lot of work if you need any help we can divide some parts of it

@FelixMalfait
Copy link
Member

Thanks Deepak! I assigned you / feel free to collaborate with Kanav of course

@FelixMalfait
Copy link
Member

For the backend you need to hit the /generate endpoint first to fill the SQLite DB with Github data

@i-am-chitti
Copy link
Contributor

@Kanav-Arora, thanks for offering the help. At the moment, I've decided to tackle this task independently. However, I'll definitely reach out if I find myself needing assistance in the future. Thanks again for your support!

@i-am-chitti
Copy link
Contributor

i-am-chitti commented Jan 19, 2024

@FelixMalfait, please confirm and clarify some doubts -

  • the new pages should be at /developers/contributors and /developers/contributors/{id}. The id would be coming from DB or should I use the login column.
image image
  • How a visitor should reach out to the contributors page? I'm not able to see any link in design either in nav bar or in footer which can direct user to this page. Maybe we can have "Contributors" in nav bar as another link.

@i-am-chitti
Copy link
Contributor

@FelixMalfait, could you please have a look at this comment - #3365 (comment) . Will need confirmation for a GO AHEAD!

@FelixMalfait
Copy link
Member

Oh sorry I had missed your comment @i-am-chitti

the new pages should be at /developers/contributors and /developers/contributors/{id}. The id would be coming from DB or should I use the login column.

yes

for selecting data, I'll be directly using SQL in any component like how it's currently done here- https://github.com/twentyhq/twenty/blob/main/packages/twenty-website/src/app/developers/contributors/page.tsx#L12-L32

yes it's not the cleanest but I wanted to start with something quick and simple. Just make sure you use prepared statements where variables are substituted by the package and not do concatenation yourself to avoid SQL injection.

I can see a Nav bar already present on this page. Though not able to find the code, probably getting from Framer via Cloudflare. So, how do I remove this in order to develop custom Nav bar as suggested above?

The navbar and footer are actually in twenty-website. I looked at the recommended way of sharing components between Framer and a React app and they basically said "don't bother just recreate an identical version and maintain both separately". We'll have a reverse proxy in front that will point to the right server based on the URL.
Let's keep the sidebar as it is now for v1 (maybe just move Docs to the right position and make it link to docs.twenty.com). We can start shipping it this way and evolve it soon after in a followup ticket.

How a visitor should reach out to the contributors page? I'm not able to see any link in design either in nav bar or in footer which can direct user to this page. Maybe we can have "Contributors" in nav bar as another link.

The idea is to have a developer hub where we'd group docs + contributors + blog with technical articles.
As a v1 we'd just have the contributors page. We can probably add it as a Resource link here first and then we'll make it more visible once we have the full hub:
Screenshot 2024-01-24 at 11 27 25

@i-am-chitti
Copy link
Contributor

@Bonapara, for background filter with gradient, there is already a gradient present on /oss-friends - https://www.twenty.com/oss-friends here. Is this an inconsistency? Do we want to have different gradient background on contributors page and oss-friends page?

@Kanav-Arora
Copy link
Contributor

Hi @FelixMalfait @Bonapara
In the merged PR card onclick we can redirect it to below url with change in author parameter
https://github.com/twentyhq/twenty/commits?author=Kanav-Arora

@FelixMalfait
Copy link
Member

@i-am-chitti yes good catch, we want the same gradient!

@Kanav-Arora Not sure what you mean? Each PR item in the list should redirect to its respective PR on Github. But maybe I didn't understand what you said

@i-am-chitti
Copy link
Contributor

@FelixMalfait ,
How should I compute rank and active days here on single contributor page -
image

Can't see any DB field dedicated to them on any table.

Also, merged PR count here is same as the number of pull request rows whose mergedAt is not NULL. That's is, it would be computed by getting list of pull requests and filtering if mergedAt is present.
image

@FelixMalfait
Copy link
Member

@i-am-chitti I was thinking we could compute the rank on the fly. I'm not sure if it needs to be cached in a column, as long as we don't have hundreds of contributors and not thousands performance should probably be okay!

Just asked ChatGPT to provide a request, it looks okay to me but I did not test it!

SELECT 
  merged_pr_counts.*,
  (RANK() OVER(ORDER BY merged_count DESC) - 1) / total_authors::float * 100 as rank_percentage
FROM
  (
   SELECT 
     authorId,
     COUNT(*) FILTER (WHERE mergedAt IS NOT NULL) as merged_count
   FROM 
     pullRequests
   GROUP BY 
     authorId
  ) AS merged_pr_counts
CROSS JOIN 
  (
   SELECT COUNT(DISTINCT authorId) as total_authors
   FROM pullRequests
  ) AS author_counts
WHERE 
  authorId = (SELECT id FROM users WHERE login = :user_id);

Also, merged PR count here is same as the number of pull request rows whose mergedAt is not NULL. That's is, it would be computed by getting list of pull requests and filtering if mergedAt is present.

Yes

Thanks!

@i-am-chitti
Copy link
Contributor

@FelixMalfait , for some reason, the above query is giving error -
image

After casting proper types of total_authors to float using CAST, the results are always 0 -

SELECT 
  merged_pr_counts.*,
  (RANK() OVER(ORDER BY merged_count DESC) - 1) / CAST( total_authors as float) * 100 as rank_percentage
FROM
  (
   SELECT 
     authorId,
     COUNT(*) FILTER (WHERE mergedAt IS NOT NULL) as merged_count
   FROM 
     pullRequests
   GROUP BY 
     authorId
  ) AS merged_pr_counts
CROSS JOIN 
  (
   SELECT COUNT(DISTINCT authorId) as total_authors
   FROM pullRequests
  ) AS author_counts
WHERE 
  authorId = (SELECT id FROM users WHERE login = :user_id)
image

I'm not able to grasp the logic for computing the rank. Could you tell the logic? I'll write down the SQL query as above AI's query is wrong, 🤔 .

@i-am-chitti
Copy link
Contributor

Also, let me know the Active Days logic too.

image

@FelixMalfait
Copy link
Member

@i-am-chitti
The first part of the query creates a table with the PR count for each author.
The second part just adds the total number of authors.
I think the issue is with the final WHERE clause, also we probably don't need the (-1). If you remove them it gives this:

Screenshot 2024-01-29 at 21 54 46

So you need to wrap that within another SQL query to apply the WHERE, e.g.
Screenshot 2024-01-29 at 21 56 34

Active Days logic should reflect the number of colored dots in the Activity section.
So everyday you authored an issue or a PR.

Also I think we want to entirely filter out core team members from everything (also when computing ranking), we'd filter this out with isEmployee=false

i-am-chitti added a commit to i-am-chitti/twenty that referenced this issue Jan 31, 2024
@i-am-chitti
Copy link
Contributor

Hey @FelixMalfait, The PR - #3745 is ready for review four days ago. I missed informing here.

@Kanav-Arora
Copy link
Contributor

@FelixMalfait @i-am-chitti
Instead of Rank you can use Top as rank can't be a decimal number

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for experienced contributor good first issue Good for newcomers scope: front Issues that are affecting the frontend side only size: short type: marketing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants