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

Contribution page #78

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

txtonylee
Copy link
Contributor

@txtonylee txtonylee commented Nov 17, 2021

Pre-Requisites

  • Yes, I updated Authors.md OR this is not my first contribution
  • Yes, I included and/or modified tests to cover relevant code OR my change is non-technical
  • Yes, I wrote this code entirely myself OR I properly attributed these changes in Third Party Notices

Description of Changes

  • A page to display contributions should be created.
  • A component needs to be built to format what a single (PR) contribution should look like when used for the Contribution page.
  • Returns a list of the current user's contribution and display it to the frontend

Related Issues

@txtonylee txtonylee marked this pull request as ready for review November 17, 2021 01:39
Copy link
Contributor

@dre8597 dre8597 left a comment

Choose a reason for hiding this comment

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

Looks good to me, just some variable name changes, and it should be good to ship 👍🏾

packages/web/src/pages/app/contributions.tsx Outdated Show resolved Hide resolved
packages/web/src/pages/app/contributions.tsx Outdated Show resolved Hide resolved
packages/web/src/pages/app/contributions.tsx Outdated Show resolved Hide resolved
packages/web/src/pages/app/contributions.tsx Outdated Show resolved Hide resolved
packages/web/src/pages/app/contributions.tsx Outdated Show resolved Hide resolved
packages/web/tests/pages/app/contributions.test.tsx Outdated Show resolved Hide resolved
packages/web/tests/pages/app/contributions.test.tsx Outdated Show resolved Hide resolved
packages/web/tests/pages/app/contributions.test.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@SpencerKaiser SpencerKaiser left a comment

Choose a reason for hiding this comment

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

More changes but making good progress!

</Box>
<Box mt="1">Score: {cbox.score}</Box>
<Box mt="1" color="gray" fontSize="xs">
{cbox.contributedAt.toString().slice(0, 10)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Pull in the DateTime component from the Luxon library and use it here (and perhaps even use relative time in a meaningful way in addition to absolute time)


export const ContributionsBox: React.FC<ContributionsBoxProps> = ({ cbox }) => {
return (
<Box maxW="sm" borderWidth="1px" borderRadius="lg" overflow="hidden" key={cbox.id}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's hop on a call and chat about this style stuff; I think there's likely a simpler way to do this using additional Chakra components

Comment on lines 22 to 25
// Get all Contributions
const res = await fetch('/api/contributions');
const ContributionsList = await res.json();

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to wrap this in a try catch and we also need to make sure that res.ok is truthy before continuing. If there's an error, we should display it meaningfully to the user via an Alert

};

describe('Contributions Box', () => {
it('renders', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking to see if it renders is not sufficient. Change this description and add more meaningful evaluations to the test below (like whether the fields within a contribution box render as expected and whether the link is clickable)

Comment on lines +36 to +37
// Wait utility
const wait = () => new Promise<void>((resolve) => setTimeout(() => resolve(), 0));
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this and use the waitForutility

packages/web/tests/pages/app/contributions.test.tsx Outdated Show resolved Hide resolved
packages/web/tests/pages/app/contributions.test.tsx Outdated Show resolved Hide resolved
packages/web/tests/pages/app/contributions.test.tsx Outdated Show resolved Hide resolved
@github-actions github-actions bot added the migration may be needed An entity file was modified, a migration may be needed label Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration may be needed An entity file was modified, a migration may be needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contribution Page Build Contribution Pull Request Box
4 participants