WestMidlands | 26-ITP-Jan | Tasleem Adedokun | Sprint 2 | Feature/form control#1101
WestMidlands | 26-ITP-Jan | Tasleem Adedokun | Sprint 2 | Feature/form control#1101tasleemadedokun wants to merge 11 commits intoCodeYourFuture:mainfrom
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
tasleemadedokun
added
the
Needs Review
tasleemadedokun
changed the title
This comment has been minimized.
This comment has been minimized.
github-actions
bot
removed
the
Needs Review
tasleemadedokun
changed the title
This comment has been minimized.
This comment has been minimized.
tasleemadedokun
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
bot
removed
the
Needs Review
tasleemadedokun
changed the title
This comment has been minimized.
This comment has been minimized.
tasleemadedokun
changed the title
This comment has been minimized.
This comment has been minimized.
tasleemadedokun
changed the title
This comment has been minimized.
This comment has been minimized.
tasleemadedokun
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
bot
removed
the
Needs Review
tasleemadedokun
changed the title
This comment has been minimized.
This comment has been minimized.
tasleemadedokun
added
Needs Review
jenny-alexander
added
Review in progress
jenny-alexander
left a comment
jenny-alexander
left a comment
There was a problem hiding this comment.
Hi @tasleemadedokun - thanks for submitting this PR. Your form has some of the basic requirements done.
There are a few requirements missing:
- The form should have a submit button in order to trigger the validation against the form. I know the form exercise guidelines don't specifically say this, but a form would normally have a submit button.
- The form exercise says to not use any CSS. Can you remove styling from your form?
https://github.com/CodeYourFuture/Module-Onboarding/tree/main/Form-Controls#developers-must-test-their-work - I left other comments in the PR for you to review.
Form-Controls/index.html
Outdated
| <fieldset> | ||
|
|
||
| <legend>CHOICE</legend> | ||
| <ul> |
There was a problem hiding this comment.
The radio buttons are styled a bit differently . What if you were to remove the list elements? Will it look more aligned?
Form-Controls/index.html
Outdated
| <label for="size"> Size</label> | ||
| <select id="size" name="size" required> | ||
| <option value=""> SELECT </option> | ||
| <option value="xtra small">XS</option> |
There was a problem hiding this comment.
For your <option> values, it's a good idea to avoid using spaces. It's technically okay in HTML but if your form data was sent to a backend system, spaces might causes headaches for systems processing the data.
So, my suggestion is to use 'kebab-case' for multi-word values (i.e. xtra small could be xtra-small).
Form-Controls/index.html
Outdated
| <footer> | ||
| <!-- change to your name--> | ||
| <h2>By HOMEWORK SOLUTION</h2> | ||
| <h2>TASLEEM ADEDOKUN</h2> |
There was a problem hiding this comment.
Using <h2> tag here implies that there is a new section starting. Since a footer name is considered minimal information, it doesn't need a heading. 🙂 Can you think of a better tag to use for your name?
There was a problem hiding this comment.
Using
<h2>tag here implies that there is a new section starting. Since a footer name is considered minimal information, it doesn't need a heading. 🙂 Can you think of a better tag to use for your name?
Hi @jenny-alexander ,
Thank you very much for taking the time to review my work. To be sincere, I had zero knowledge of coding until I joined CYF. I am trying my best, and I am glad you spaced the correction.
However, I have a question? Before I added the CSS, my Lighthouse score was not 100%. Can I get 100% without CSS?
Thanks once again.
There was a problem hiding this comment.
You're doing a great job 😀
Absolutely you can get a 100% accessibility score on Lighthouse without using CSS. What accessibility issues were you getting? Did you try to ask AI (ChatGPT, Gemini, Claude, etc) about the accessibility issue?
I can also help if you aren't able to figure it out.
There was a problem hiding this comment.
Hi @jenny-alexander , thank you very much i have done all neccesary changes.
jenny-alexander
added
Reviewed
|
Thank you very much. Sometimes I need this encouragement. My accessibility
was 93%. AI gave me the impression that accessibility can't be 100% without
adding even if its a line of css. Now I know better. I would try and work
on all changes mentioned ABOVED.
…On Thu, 5 Feb 2026, 14:19 Jennifer Alexander, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In Form-Controls/index.html
<#1101 (comment)>
:
> </form>
</main>
<footer>
- <!-- change to your name-->
- <h2>By HOMEWORK SOLUTION</h2>
+ <h2>TASLEEM ADEDOKUN</h2>
You're doing a great job 😀
Absolutely you can get a 100% accessibility score on Lighthouse without
using CSS. What accessibility issues were you getting? Did you try to ask
AI (ChatGPT, Gemini, Claude, etc) about the accessibility issue?
I can also help if you aren't able to figure it out.
—
Reply to this email directly, view it on GitHub
<#1101 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/B2G4CKQHDDCPRWC47LUKXM34KNGONAVCNFSM6AAAAACTQVXCLWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTONJXGQYTSMRRHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
tasleemadedokun
added
Needs Review
jenny-alexander
left a comment
jenny-alexander
left a comment
There was a problem hiding this comment.
Can you review your usage of the <p> tag in your form? I believe you might be using for styling purposes to give your form more space.
Since this tag is used to represent a paragraph, it doesn't make semantic sense to use it to wrap a form label or a button.
Here is a resource you can review about <p>: https://www.w3schools.com/tags/tag_P.asp
It's okay for the form to look unstyled since the exercise requirements state to not use CSS. 🙂
| </form> | ||
| </main> | ||
| <p> | ||
| <button>SUBMIT</button> |
There was a problem hiding this comment.
I see you added the submit button. This is good however the submit button is currently outside of the form.
In order for the form validation (i.e. required fields) to be triggered a submit button has to be within the form.
Can you try to make that change and then ensure that when you click on the button, you get a message if you leave required fields empty.
| this will also help you fill in your PR message later--> | ||
| <p>Please complete all required (*) fields</p> | ||
| <section> | ||
| <P> |
There was a problem hiding this comment.
Should the tag be <p> or <P>? Can you research online and find the correct casing?
| <legend>CHOICE</legend> | ||
|
|
||
| <label> | ||
| <input type="radio"name="choice" value="G" required> GREEN |
There was a problem hiding this comment.
I see a space missing between type and name.
|
Nice, accessibility is 100%. 🙌 |
jenny-alexander
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
I have prepared a form page using the README instructions and I have also made some commit and added css to make the accessibility 100%