Skip to content

London | May-2025 | Samuel Tarawally | Form Controls #689

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 4 commits into
base: main
Choose a base branch
from

Conversation

Tarawally
Copy link

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

Changelist

This PR updates index.html to align with README.md requirements.

Key Changes:

  • Pure HTML Structure: All styling has been removed, making the form entirely HTML driven.
  • Input Validation: Ensures all required fields (name, email, colour, size) are correctly validated.
  • User Accessibility: All form inputs are properly labelled, improving usability for all users.

Copy link

netlify bot commented Jun 3, 2025

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit e89d5e0
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/68407bfdfd6dbd00084a424b
😎 Deploy Preview https://deploy-preview-689--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: 91 (🟢 up 5 from production)
PWA: -
View the detailed breakdown and full score reports

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

@Tarawally Tarawally added the Needs Review Participant to add when requesting review label Jun 3, 2025
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is clean, error-free, and properly formatted. The implementation is robust. The pull request description is clear, concise. Excellent job!

The Lighthouse Accessibility score is not yet 100. Can you improve the score to 100?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Jun 3, 2025
@Tarawally
Copy link
Author

I appreciate the quick review and feedback, @cjyuan. I'll revisit the Lighthouse Accessibility score to see what's going on and make adjustments are possible.

@Tarawally Tarawally added Needs Review Participant to add when requesting review and removed Reviewed Volunteer to add when completing a review labels Jun 4, 2025
@Tarawally
Copy link
Author

I've made the following adjustments to achieve a 100% Lighthouse accessibility score. This involved:

  • Improved form structure by replacing <div> with <p> for better semantic grouping and spacing.

@cjyuan
Copy link
Contributor

cjyuan commented Jun 4, 2025

The content within <p> is not really a paragraph. Semantically <div> is better (<div> has no semantic).
Without using CSS, we can create vertical gap using <br> (line break). (Sort of a hack).
This is a simple change which I am sure you can make.

I will go ahead and mark this PR as "Complete".

The key takeaway is that we should check everything thoroughly — especially those checks that can be performed using automated tools — before submitting our work for review.

@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 labels Jun 4, 2025
@Tarawally Tarawally linked an issue Jun 6, 2025 that may be closed by this pull request
3 tasks
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.

03 Form Controls 03 Form Controls
2 participants