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] GitHub Component Kit #91

Merged
merged 21 commits into from
Mar 26, 2022

Conversation

samad-yar-khan
Copy link
Contributor

@samad-yar-khan samad-yar-khan commented Mar 13, 2022

Issues

Mentioned in the issue #90

Proposed changes (including videos or screenshots)

  • Improved database schema for re-usability of component for different repositories. image
  • Improved data fetching method in cms (can be re-used when switching from strapi) to provide a better experience for community builders. All data fetching for the github kit for a repository will be done by a a single githubKit() function.
  • Replacing individual component tags with a <Github ... > tag which can be used to fetch different components of the kit depending on props/parameters. <Github type={'contributors'} githubData={props.githubData} />
  • Improve github helper functions (which fetch data from the cms into our application) into a single githubKitData(owner ,name ,[..needs]) function which can fetch relevant data based on the components we wish to use on a particular page.
  • New Pull Request Component


    crons-example

  • New Repository overview component


    crons-example

  • Updated Documentation . P.S. Refer to the documentation for changes in usability.

@lgtm-com
Copy link

lgtm-com bot commented Mar 13, 2022

This pull request introduces 5 alerts when merging 2f888d7 into 50f7da2 - view on LGTM.com

new alerts:

  • 4 for Unused variable, import, function or class
  • 1 for Overwritten property

@lgtm-com
Copy link

lgtm-com bot commented Mar 13, 2022

This pull request introduces 1 alert when merging 7831f14 into 50f7da2 - view on LGTM.com

new alerts:

  • 1 for Overwritten property

@samad-yar-khan
Copy link
Contributor Author

@Sing-Li I have made some changes and removed the unnecessary code, it would be great if you could review the pull request.

Copy link
Member

@Sing-Li Sing-Li left a comment

Choose a reason for hiding this comment

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

Good clean up work, congrats.

However, you added back package.json and some index.js modifications

Please remove package.json and the index.js modifications. Take a look at other PRs and see how they use a demo page under pages to demonstrate their component. Thanks.

@samad-yar-khan
Copy link
Contributor Author

samad-yar-khan commented Mar 18, 2022

@Sing-Li I have made the requested changed :

  • Removed the changes to the index.js file.
  • Created a demo page for the Github Component Kit which can be accessed on the route /github .
  • Removed .package-lock.json file .
2022-03-18.18-29-21.mp4

@Sing-Li Sing-Li merged commit 0930180 into RocketChat:master Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants