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.

Sign up for GitHub

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

Jump to bottom

feat/add-guide-footer #35567

Merged
merged 2 commits into from
May 2, 2019
Merged

feat/add-guide-footer #35567

merged 2 commits into from
May 2, 2019

Conversation

moT01
Copy link
Member

@moT01 moT01 commented Mar 11, 2019

This adds a footer to all guide articles with instructions on how to contribute to the article.

  • I have read freeCodeCamp's contribution guidelines.
  • My pull request has a descriptive title (not a vague title like Update index.md)
  • My pull request targets the master branch of freeCodeCamp.
  • None of my changes are plagiarized from another source without proper attribution.
  • All the files I changed are in the same world language (for example: only English changes, or only Chinese changes, etc.)
  • My changes do not use shortened URLs or affiliate links.

This doesn't exactly close the issue, check it out for more details...
related to #35490

@moT01 moT01 requested a review from a team March 11, 2019 13:44
@raisedadead raisedadead added status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. labels Mar 11, 2019
@raisedadead
Copy link
Member

@moT01 how is this coming along?

@moT01
Copy link
Member Author

moT01 commented Mar 29, 2019

Slowly, I got sidetracked and kind of put it on the back burner, but didn't forget about it.

@moT01 moT01 force-pushed the feat/add-guide-footerish branch 2 times, most recently from 9ec9f62 to aeb3a45 Compare April 3, 2019 18:27
@moT01
Copy link
Member Author

moT01 commented Apr 3, 2019 •
edited

Work in progress - any push in the right direction is appreciated.

@moT01
Copy link
Member Author

moT01 commented Apr 3, 2019 •
edited

okay, I pushed another new solution. I think I got it right this time. I might need to do a little clean up.

Edit: scratch that - getting an error

raisedadead
raisedadead reviewed Apr 4, 2019
View reviewed changes
Copy link
Member

@raisedadead raisedadead left a comment •
edited

Choose a reason for hiding this comment

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

Hi @moT01

I just ran through the code on GitHub and seems you are there. Do you mind cleaning up the commits and squash them for me to pull down and review?

Making them conventional would be ideal while you are at it.

Thanks a lot for all your hard work on this.

@moT01 moT01 force-pushed the feat/add-guide-footerish branch 2 times, most recently from 5de79be to ab53e77 Compare April 4, 2019 13:36
@moT01
Copy link
Member Author

moT01 commented Apr 4, 2019

✔️ ?

@raisedadead raisedadead force-pushed the feat/add-guide-footerish branch 2 times, most recently from 27f284f to 2511832 Compare April 4, 2019 17:53
@raisedadead raisedadead removed the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Apr 4, 2019
raisedadead
raisedadead approved these changes Apr 4, 2019
View reviewed changes
Copy link
Member

@raisedadead raisedadead left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks @moT01 , I have gone ahead and made a few cosmetic tweaks to the footer text. Waiting for the CI and a second pair of eyes before merging.

@raisedadead raisedadead added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Apr 4, 2019
@moT01 moT01 requested a review from ValeraS May 1, 2019 14:26
ValeraS
ValeraS suggested changes May 1, 2019
View reviewed changes
client/src/templates/Guide/components/GuideFooter.js Outdated
const propTypes = {
githubPath: PropTypes.string
};
class GuideFooter extends React.Component {
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 redo this to a function component

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe the new changes should resolve this.

client/src/templates/Guide/GuideArticle.js Outdated
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
import { graphql } from 'gatsby';

import ArticleLayout from './components/ArticleLayout';
import GuideFooter from './components/GuideFooter';
Copy link
Contributor

Choose a reason for hiding this comment

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

How about posting this in an article layout?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't exactly sure what you meant here, I took it as moving the GuideFooter component to the ArticleLayout component. So I did that, let me know if it's not what you wanted.

@moT01 moT01 requested a review from QuincyLarson as a code owner May 1, 2019 16:48
@moT01 moT01 removed the request for review from QuincyLarson May 1, 2019 16:56
@moT01 moT01 force-pushed the feat/add-guide-footerish branch from a1e47c7 to f023d43 Compare May 1, 2019 17:16
@moT01 moT01 force-pushed the feat/add-guide-footerish branch from f023d43 to be24f19 Compare May 1, 2019 17:25
@moT01
Copy link
Member Author

moT01 commented May 1, 2019

Thanks for the review @ValeraS - After some struggles with git, I think I got it how you want. I left comments on your review things there.

@raisedadead raisedadead self-requested a review May 2, 2019 07:55
@raisedadead raisedadead merged commit 1660a26 into freeCodeCamp:master May 2, 2019
@moT01 moT01 deleted the feat/add-guide-footerish branch May 2, 2019 23:05
@moT01 moT01 mentioned this pull request May 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raisedadead raisedadead raisedadead approved these changes

@ValeraS ValeraS ValeraS approved these changes

Assignees
No one assigned
Labels
platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@moT01 @raisedadead @ValeraS