Change Requests
We received a code review so we're again on the right side of the roadmap. The first question changes requested? can be answered with yes.
As for the second question: There's currently no active branch beside the footer
branch. That means no branch can depend on the code inside the footer
branch. So the answer to does a branch depend on the new changes? is no.
Thus the flow looks like this.
The first step is to check out the PR branch (meaning footer
). At this point, you should still be on that branch. Otherwise, check it out.
Note: if you're not sure which branch you're on you can run
git status
orgit branch
and check the output.
The next step on the roadmap is commit and push changes.
Change 1: Show footer at the bottom
Your teammate is right. When you run the development server with npm start
and visit the page you can see that the footer is directly below the header. Doesn't look good, right?
Let's fix this. Open the file src/index.html
. We'll add an element between the header and footer that takes up the required space to place the footer at the bottom.
<!doctype html> <html lang="en"><head> <meta charset="utf-8"> <title>GitHub Flow Course</title> <meta name="description" content="GitHub Flow Course"> <meta name="author" content="Johannes Kettmann"> <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/normalize/8.0.1/normalize.min.css"> <link rel="stylesheet" href="styles.css"></head> <body> <header class="header"> Header </header> <main class="content"></main> <footer class="footer"> Footer </footer></body></html>
Next, open the file src/styles.css
and add the .content
class.
body { font-family: Arial, Helvetica, sans-serif; font-size: 18px;} .header { height: 50px; display: flex; justify-content: center; align-items: center; border-bottom: 1px solid black;} .footer { height: 50px; display: flex; justify-content: center; align-items: center; border-top: 1px solid black;} .content { min-height: calc(100vh - 100px);}
Now commit these changes before you continue.
Change 2: Fixing the footer height
We've seen it already in the fix for the header. The height of the footer has to be 1px
less since it has a top border.
Open the file src/styles.css
again and change the footer height to 49px
.
body { font-family: Arial, Helvetica, sans-serif; font-size: 18px;} .header { height: 50px; display: flex; justify-content: center; align-items: center; border-bottom: 1px solid black;} .footer { height: 49px; display: flex; justify-content: center; align-items: center; border-top: 1px solid black;} .content { min-height: calc(100vh - 100px);}
Great. Now commit this change as well.
Finally, push the new commits to GitHub.
Re-request a review
The last step on the roadmap is request new review.
Inside the Pull Request on GitHub, you can see a refresh 🔄 icon that you can click to re-request a review.
You should get feedback by ooloo-bot
after a few seconds.
Note: Feel free to resolve the conversations here by clicking the button in the screenshot below. In a real team I'd leave that up to the reviewer though.