Skip to content

Glasgow|May 2025|Indira Muniappan|Form controls #688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ind222
Copy link

@ind222 ind222 commented Jun 3, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason

  • I have titled my PR with REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME

  • I have tested my changes

  • My changes follow the style guide

  • My changes meet the requirements of this task

Change list

  • Fixed HTML validator warnings
  • Removed inline CSS as per requirement
  • Achieved Lighthouse accessibility score of 100

Questions

None at the moment.

Copy link

netlify bot commented Jun 3, 2025

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 6e67dc8
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/6842109e92439b0008523842
😎 Deploy Preview https://deploy-preview-688--cyf-onboarding-module.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
2 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (🟢 up 14 from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@ind222 ind222 added the Needs Review Participant to add when requesting review label Jun 3, 2025
@cjyuan cjyuan added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Participant to add when requesting review labels Jun 3, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Jun 3, 2025

Your codes is error-free and your form implementation is solid. Excellent job!

I only have a suggestion: Can you try asking ChatGPT

Check consistency:
...  <-- Paste your code here

to see if there’s anything (no matter how minor) that could be improved?


On separate note: Can you update your PR title to match the format
REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME

  • Use | as separators between adjacent fields
  • Set "PROJ_NAME" to Form Controls (no need to mention the module name; this PR is created in the "Module-Onboarding" repo)

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jun 3, 2025
@ind222
Copy link
Author

ind222 commented Jun 3, 2025

Hi CJYuan,
Thank you for your incredible support.
I have Updated my PR title to match the format.

I consulted ChatGPT for a consistency review and received helpful recommendations, including wrapping the in a , reducing redundant aria-required attributes, using consistent container tags for form groups, and ensuring uniform structure by wrapping each input-label pair in a . These insights have improved my understanding and helped me enhance the code quality. Kindly let me know if there are any inputs or updates needed to complete my PR. Warm regards Indira.

@cjyuan
Copy link
Contributor

cjyuan commented Jun 3, 2025

  1. The PR title I was referring to is this one:
    image
    You can click the "Edit" on the right to edit the title.

  2. If you have made any changes to your code on your computer, you need to commit the changes and push the commit to Github. Currently I don't see any new commit on this branch since my previous review.

  3. The comment in Github uses Markdown syntax. Any HTML code typed in the comment is rendered as is unless it is formatted as a code block. That's why <select> and <p> in your response are not rendered in your message.

@cjyuan
Copy link
Contributor

cjyuan commented Jun 3, 2025

Please note that in CYF courses, the recommended way to inform the reviewer of your changes is to do both of the following:

  • Reply to their feedback. (which you have done -- great!)
    • In the responses, clarify how each piece of feedback was addressed to demonstrate that you've carefully reviewed the suggestions.
    • Your response may trigger a notification (depending on the reviewer's settings), helping ensure they’re aware of the updates you’ve made.
  • Replace the "Reviewed" label by a "Needs review" label
    • Without this label, the reviewer would not know if your changes is ready to be reviewed.

@ind222
Copy link
Author

ind222 commented Jun 3, 2025

Hi CJ,
As recommended I use Edit button for changing PR title. I Hope it meets CYF requirement .
I didn’t made any change in the code and there is no new commit as well.
I am grateful to know of Markdown syntax and i will make it useful for my future project.
Your feedback and suggestions improves my knowledge.
kindly let me know inputs and updates for my PR completions.
Warm regards
Indira.

@ind222 ind222 added the Needs Review Participant to add when requesting review label Jun 3, 2025
@ind222 ind222 changed the title Glasgow_May 2025_Indira Muniappan_Module Onboarding _Form controls Glasgow|May 2025|Indira Muniappan|Form controls Jun 3, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Jun 3, 2025

Changes look good.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and review comments have been addressed and removed Needs Review Participant to add when requesting review Reviewed Volunteer to add when completing a review labels Jun 3, 2025
@ind222
Copy link
Author

ind222 commented Jun 3, 2025

Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and review comments have been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants