Skip to content
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

London_cohort| Aung Ye Kyaw | Module-Onboarding | Week1|Formcontrol #263

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

Conversation

sarawone
Copy link

Form control exercise

  1. I have used HTML only.
  2. I have not used any CSS or JavaScript.
  3. My form is semantic html.
  4. All inputs have associated labels.
  5. My Lighthouse Accessibility score is 100.
  6. I require a valid name. I have defined a valid name as a text string of two characters or more.
  7. I require a valid email.
  8. I require one colour from a defined set of 3 colours.
  9. I require one size from a defined set of 6 sizes.

uploaded on the wrong branch
uploaded on the wrong branch
uploaded on the wrong branch
creating form controls
Copy link

netlify bot commented Feb 12, 2025

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 6a62a19
🔍 Latest deploy log https://app.netlify.com/sites/cyf-onboarding-module/deploys/67ad3a97393a690008fff15d
😎 Deploy Preview https://deploy-preview-263--cyf-onboarding-module.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

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

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

@sarawone sarawone added the Needs Review Participant to add when requesting review label Feb 12, 2025
@day-lee day-lee 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 Feb 12, 2025
@day-lee
Copy link

day-lee commented Feb 12, 2025

Good job! Your code is easy to read and looks good overall.

I have a few suggestions:

  1. When I type one character for the name input, it still accepts the value. The current regex pattern requires one or more ("+") characters. Consider a way to ensure a valid name is a text string of two characters or more. You could use a regex pattern or set a minimum length attribute.

  2. The name, email, and size fields are wrapped with p tags. While the p tag is used for grouping content, it is typically intended for blocks of text. Can you change the code to make it semantically more appropriate?

  3. The current accessibility score is 93. I see there is room for improvement. Can you change the code so it can achieve a score of 100?

Keep up the good work!

@day-lee day-lee 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 Feb 12, 2025
@day-lee day-lee self-requested a review February 13, 2025 12:00
@day-lee day-lee added Complete Participant to add when work is complete and review comments have been addressed and removed Reviewed Volunteer to add when completing a review labels Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Participant 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