twenty icon indicating copy to clipboard operation
twenty copied to clipboard

New Contributors Page for Twenty Website

Open Bonapara opened this issue 1 year ago • 20 comments

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;)

https://github.com/twentyhq/twenty/assets/19412894/e8bd70d9-b71c-4530-95f4-4526be79798c

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:

https://github.com/twentyhq/twenty/assets/19412894/8973441c-1d0a-463b-8ff5-1da201dd1c0e

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 ;)

Bonapara avatar Jan 11 '24 09:01 Bonapara

@Bonapara ofcourse not

Kanav-Arora avatar Jan 11 '24 10:01 Kanav-Arora

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.

FelixMalfait avatar Jan 11 '24 11:01 FelixMalfait

@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

Kanav-Arora avatar Jan 11 '24 11:01 Kanav-Arora

@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.

FelixMalfait avatar Jan 11 '24 11:01 FelixMalfait

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

i-am-chitti avatar Jan 18 '24 17:01 i-am-chitti

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

Kanav-Arora avatar Jan 18 '24 17:01 Kanav-Arora

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

FelixMalfait avatar Jan 18 '24 20:01 FelixMalfait

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

FelixMalfait avatar Jan 18 '24 20:01 FelixMalfait

@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 avatar Jan 19 '24 18:01 i-am-chitti

@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

  • 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

  • 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? 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 avatar Jan 19 '24 18:01 i-am-chitti

@FelixMalfait, could you please have a look at this comment - https://github.com/twentyhq/twenty/issues/3365#issuecomment-1900865919 . Will need confirmation for a GO AHEAD!

i-am-chitti avatar Jan 23 '24 21:01 i-am-chitti

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

FelixMalfait avatar Jan 24 '24 10:01 FelixMalfait

@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?

i-am-chitti avatar Jan 25 '24 20:01 i-am-chitti

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

Kanav-Arora avatar Jan 26 '24 05:01 Kanav-Arora

@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

FelixMalfait avatar Jan 26 '24 09:01 FelixMalfait

@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

i-am-chitti avatar Jan 28 '24 16:01 i-am-chitti

@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!

FelixMalfait avatar Jan 28 '24 20:01 FelixMalfait

@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 avatar Jan 29 '24 19:01 i-am-chitti

Also, let me know the Active Days logic too.

image

i-am-chitti avatar Jan 29 '24 19:01 i-am-chitti

@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

FelixMalfait avatar Jan 29 '24 20:01 FelixMalfait

Hey @FelixMalfait, The PR - https://github.com/twentyhq/twenty/pull/3745 is ready for review four days ago. I missed informing here.

i-am-chitti avatar Feb 04 '24 11:02 i-am-chitti

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

Kanav-Arora avatar Feb 04 '24 17:02 Kanav-Arora