-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
Added important links button to the organization information page #144
Conversation
@nishantwrp please review this PR and let me know if this is mergeable or not OR should I create a merge request with master branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pnishant23 for taking this up! Took an initial pass, will take a more thorough look once the deploy preview is ready.
src/components/org-info.jsx
Outdated
@@ -177,6 +178,25 @@ const OrgInfo = ({ data }) => { | |||
} | |||
/> | |||
)} | |||
{data.links && | |||
data?.links?.map(item => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't need optional chaining
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, will remove that
package-lock.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can revert this change
api/compile-data.js
Outdated
@@ -241,7 +245,18 @@ const compileData = () => { | |||
const distinctOrganizations = organizationSet.extract() | |||
const gsocOrganizations = [] | |||
distinctOrganizations.forEach(orgList => { | |||
gsocOrganizations.push(getCombinedOrgJson(orgList)) | |||
// ---------new code ------------ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please clean these comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have removed this comments form the file
Just to give some context, I was trying to get netlify deploy preview to work. Seems like changes to package-lock.json & yarn-lock.json need to be reverted. |
I have removed the comments from this file
✅ Deploy Preview for gsoc-organizations ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I have removed optional chaining, hope this helps
closing this PR because created an updated PR for, which is PR 148 |
In the organization information page we have the buttons which directs to various link of a particular organization. However not all the important links are getting fetched from the Open API. Hence those missing important links have been added to the imp_links.json file created by @nishantwrp and I have fetched that file in the compile-data.js file which feeds the data to organization information page.
To added these important links head towards the imp_links.json file in /api folder and added the json object.
Guide to add the links
@nishantwrp, @beingnoble03 please review the code and let me know. If everything seems to be alright do merge the code so that this feature can be implemented on the live site