Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Mugs3
added
the
Needs Review
There was a problem hiding this comment.
Good attempt. A few things to sort before it can be marked Complete:
- Name validation. The task asks for at least two non-space characters and the README gives you the regex to use. At the moment a single letter or two spaces goes through. See line 18.
- The task says not to write a form action. This form has one, and on the deploy preview pressing Place Order sends the customer to a page that doesn't exist. See line 15.
- Reload the page and press Place Order with valid name and email. It goes through with Red and XS, which the customer never chose. This is also two of the three errors the HTML validator from the README checklist (https://validator.w3.org/) reports. See line 38.
- There's a stray
</div>on line 54. It's the validator's third error and Prettier can't parse the file because of it. - Customer Number isn't part of the task. The README says we already have the customer's details and only need name, email, colour and size. See line 22.
style.cssisn't linked from the page, so none of it applies. What goes in the<head>to load a stylesheet?- The footer still says HOMEWORK SOLUTION, line 62.
- Formatting. Once the structure is fixed, run
npx prettier --write Form-Controlsfrom the repo root and push.
Add the Needs Review label again once you've pushed.
| <form action="order" method="post"> | ||
| <div> | ||
| <label for="CustomerName">Customer Name:</label> | ||
| <input type="text" id="CustomerName" name="CustomerName" required> |
There was a problem hiding this comment.
required only stops the field being empty. Type two spaces and it passes. The README gives you a regex for "at least two non-space characters". Which attribute does it go in?
There was a problem hiding this comment.
Fixed, and two spaces are rejected now. Good.
| <!-- | ||
| try writing out the requirements first as comments | ||
| this will also help you fill in your PR message later--> | ||
| <form action="order" method="post"> |
There was a problem hiding this comment.
The task says "Do not write a form action for this project". What happens on the deploy preview when you fill the form in correctly and press Place Order?
There was a problem hiding this comment.
Action gone. One thing left on this line, see the new comment.
| <div> | ||
| <label for="T-ShirtsColor">T-Shirt Color:</label> | ||
| <select id="T-ShirtsColor" name="T-ShirtsColor" required> | ||
| <option value="Red">Red</option> |
There was a problem hiding this comment.
Red is selected as soon as the page loads, so required has nothing to check. Read the validator's message for this line. What does it say the first option needs? Same for the size select on line 46.
There was a problem hiding this comment.
Both dropdowns fixed. Good.
| <option value="XXL">XXL</option> | ||
| </select> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Which <div> is this closing? Count the opening and closing divs above it.
| </div> | ||
| <div> | ||
| <label for="CustomerNumber">Customer Number:</label> | ||
| <input type="number" id="CustomerNumber" name="CustomerNumber" required> |
There was a problem hiding this comment.
Where in the task is a customer number asked for? Every extra required field is one more thing the customer has to get right before they can order.
abdishakoor-dev
added
Reviewed
Mugs3
added
Needs Review
There was a problem hiding this comment.
Nearly all of it is done: the name check works, both dropdowns now insist on a choice, the stray </div> is gone, Customer Number is out, the stylesheet loads and your name is in the footer. The validator is clean. Two things left before I can mark this Complete:
method="post"is still on the form tag, so pressing Place Order sends the customer to a "page not found". Make it plain<form>like the starter file. See line 16.- The formatting check. "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
index.htmlandstyle.cssfail 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
| try writing out the requirements first as comments | ||
| this will also help you fill in your PR message later--> | ||
| </form> | ||
| <form method="post"> |
There was a problem hiding this comment.
action has gone, good. method="post" should go too. On a real site a form does have an action and a method, that's how the data gets to a server, and you'd be right to reach for them. This exercise is the exception: the task says not to write a form action because there's no server to send to, and with method="post" left on the browser tries to send anyway, which is why you land on "page not found". Make the tag plain <form> like the starter file. The Place Order button's only job here is to make the browser run the validation.
There was a problem hiding this comment.
Still method="post" on GitHub. Make it plain <form>, no method at all.
abdishakoor-dev
added
Reviewed
Mugs3
added
Needs Review
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Your commit message says both files were formatted and the form method changed, but what has arrived on GitHub is only part of that: index.html is formatted (good), style.css hasn't changed at all and still fails Prettier, and line 16 still reads <form method="post">.
Two likely causes: the changes to style.css and the form tag weren't saved before you committed, or they were saved but not staged. In VS Code, look at the Source Control panel after you save; any file still listed under Changes hasn't been committed yet.
Two things left, same as before:
- Line 16: the form tag should be plain
<form>, with no method either (notget), since this form doesn't send anywhere. - Format style.css.
Save, check Source Control shows nothing outstanding, commit, push, and add the Needs Review label again.
abdishakoor-dev
added
Reviewed
2 participants
Self checklist
Task code
CYF-1004
Changelist
15 Sept:
I have formatted both documents using prettier and changed form method to get as fix for page not found error on clicking place order button
14 Sept:
I have updated and fixed issues raised by the reviewer and linked the style.css file to the index file
13 Sept
I have updated the Form-Control file/index file to design and implement the user requirements into a usable system
I have also added the css file to make the system to improve the look and feel of the system
I agree to follow the code of conduct for this organisation.