Glasgow| 26-ITP-Jan|Fattouma Ouannassi| Sprint 1 | Wireframe/coursework#999
Open
FAprogrammerO wants to merge 2 commits intoCodeYourFuture:mainfrom
Open
Glasgow| 26-ITP-Jan|Fattouma Ouannassi| Sprint 1 | Wireframe/coursework#999FAprogrammerO wants to merge 2 commits intoCodeYourFuture:mainfrom
FAprogrammerO wants to merge 2 commits intoCodeYourFuture:mainfrom
Conversation
<!-- You must title your PR like this: Region | Cohort | FirstName LastName | Sprint | Assignment Title For example, London | 25-ITP-May | Carol Owen | Sprint 1 | Alarm Clock Fill in the template below - remove any sections that don't apply. Complete the self checklist - replace each empty box in the checklist [ ] with a [x]. Add the label "Needs Review" and you will get review. Respond to volunteer reviews until the volunteer marks it as "Complete". --> ## Learners, PR Template Self checklist - [x] I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title - [x] My changes meet the requirements of the task - [x] I have tested my changes - [x] My changes follow the [style guide](https://curriculum.codeyourfuture.io/guides/reviewing/style-guide/) ## Changelist This PR includes modifications to the HTML and CSS to create a webpage that follows the provided wireframe layout. The webpage explains the purpose of a README file and the wireframe, aswell as the meaning of a branch in Git, with each topic presented as an article section. ## Questions /
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
FAprogrammerO
added
Needs Review
jenny-alexander
added
Review in progress
Contributor
jenny-alexander
left a comment
jenny-alexander
left a comment
There was a problem hiding this comment.
Hey @FAprogrammerO - you put in some really good work for this wireframe exercise!
There are a few adjustments that should be made, including:
- Can you run your page through W3 validator: https://validator.w3.org/
I see an error in your html and this validator will display it to you. - It's a good idea to format your code so it's easier to read and maintain. You can do this in VSCode, by right-clicking within
index.htmland choosingFormat Document.
Wireframe/index.html
Outdated
| <title>Wireframe to Web Code|Coursework</title> | ||
| <link rel="stylesheet" href="style.css" /> | ||
|
|
||
| <style> |
Contributor
There was a problem hiding this comment.
According to the exercise acceptance criteria, all of the styling should be done within the CSS file. I see you added styling within the index.html file. Can you make the adjustment?
Wireframe/index.html
Outdated
| <a href="">Read more</a> | ||
| </article> | ||
| </main> | ||
| <section class="article-block"> |
Contributor
There was a problem hiding this comment.
I noticed you switched out the <article> for <section>. Was there a reason for doing this?
Author
There was a problem hiding this comment.
I switched from article for section to group the articles together, but now after you mentioned that I found out that it wasn't necessary.
| Lorem ipsum dolor sit amet consectetur adipisicing elit. Quisquam, | ||
| voluptates. Quisquam, voluptates. | ||
| </p> | ||
| <a href="">Read more</a> |
Contributor
There was a problem hiding this comment.
According to the wireframe exercise, each article should have a 'Read more' link (styled as a button) that will bring the user to an article about that topic. Can you add this back?
jenny-alexander
added
Reviewed
FAprogrammerO
added
Needs Review
Contributor
|
@FAprogrammerO - Great job incorporating code feedback! Your page looks great. 🙌 |
jenny-alexander
added
Complete
Author
|
Thank you so much @jenny-alexander for your time, and provide such valuable feedback, your suggestions really helped me improve my page 🙏🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Learners, PR Template
Self checklist
Changelist
This PR includes modifications to the HTML and CSS to create a webpage that follows the provided wireframe layout.
The webpage explains the purpose of a README file and the wireframe, aswell as the meaning of a branch in Git, with each topic presented as an article section.
Questions
/