London | 26-ITP-Sep | Shirin Panahian | Sprint 1| Form Controls - #1496
shirinpanahian wants to merge 10 commits into
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
miriamjorna
added
the
Needs Review
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
There was a problem hiding this comment.
All the requirements are met and the form works. Good commits too, one per step.
One thing to sort before I mark this Complete: Prettier fails on both files. Run npx prettier --write Form-Controls from the repo root and push.
Two small inline notes as well, neither a blocker. Add the Needs Review label again once you've pushed.
| font-size: 16px; | ||
| } | ||
| button{ | ||
| padding: 12px,24px; |
There was a problem hiding this comment.
There's a comma between the two values, which isn't valid CSS, so the browser drops this rule and the button keeps the 10px padding from the rule above. What separates values in a shorthand property?
There was a problem hiding this comment.
I corrected it. In CSS shorthand properties, values are separated by spaces, not commas.
There was a problem hiding this comment.
Correct, and the button now gets its padding.
| </form> | ||
| </main> | ||
| <footer> | ||
| <!-- change to your name--> |
There was a problem hiding this comment.
This comment was an instruction from the starter file. Now you've done it, it can go.
There was a problem hiding this comment.
Hi, I’ve fixed the formatting in both files using Prettier and removed the starter comment as requested. I’ve also committed and pushed the changes. Please have another look when you have a chance. Thank you!
There was a problem hiding this comment.
Seen, thanks. Marking it Complete.
abdishakoor-dev
added
Reviewed
shirinpanahian
added
the
Needs Review
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
There was a problem hiding this comment.
The padding fix is right. The formatting check is the one thing still outstanding. "My code is consistently formatted" is on the README checklist, and the tool that does it for you is called Prettier. It rearranges spacing and indentation to one agreed style, so that your code is easy to read and so that a reviewer only sees the changes you meant to make, not stray spaces and tabs. At the moment both of your files fail that check.
Prettier comes with the CYF extension pack you were asked to install during onboarding. If you're not sure you have it, open VS Code, go to Extensions, and search for CodeYourFuture Extension Pack; install it if it isn't there: https://marketplace.visualstudio.com/items?itemName=CodeYourFuture.cyf-extension-pack
Then open each of your two files, right click in the editor, choose Format Document, and pick Prettier if VS Code asks which formatter to use. Save, commit the changes it makes, push, and add the Needs Review label again. To make this happen automatically every time you save, follow the format on save steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md
The starter comment on line 52 can go in the same commit.
abdishakoor-dev
removed
the
Needs Review
shirinpanahian
added
the
Needs Review
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Both files formatted and the comment gone. All done, marking this Complete. Well done Shirin.
abdishakoor-dev
added
Complete
3 participants
Learners, PR Template
Self checklist
Task code
CYF-1004
Changelist
I am creating a T-shirt order form using HTML and CSS. I put four field name, email, T-shirt colour and size.