Skip to content

Glasgow | May-2026 | Craig Stoddart | Sprint 1 | Form Controls #1413

Open
CraigStodd wants to merge 17 commits into
CodeYourFuture:mainfrom
CraigStodd:feature/form-control
Open

Glasgow | May-2026 | Craig Stoddart | Sprint 1 | Form Controls #1413
CraigStodd wants to merge 17 commits into
CodeYourFuture:mainfrom
CraigStodd:feature/form-control

Conversation

@CraigStodd
Copy link
Copy Markdown

@CraigStodd CraigStodd commented Jun 4, 2026

LINK TO PREVIOUS PR:

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

Updated to include changes suggested in previous pull request.
Implemented changes that meet all requirements for the Form Control exercise

@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 4, 2026

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 537aeb3
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/6a217266ab8d4b0008ff0e27
😎 Deploy Preview https://deploy-preview-1413--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: 86 (no change from production)
PWA: -
View the detailed breakdown and full score reports
🤖 Make changes Run an agent on this branch

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

@CraigStodd CraigStodd added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jun 4, 2026
@CraigStodd
Copy link
Copy Markdown
Author

Hi @cjyuan, here is the updated PR with the changes you suggested.

Copy link
Copy Markdown
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.

Changes look good.

I only a few more minor requests.

Comment thread Form-Controls/index.html
Comment on lines +69 to +72
<label
>XS:
<input type="radio" name="Size" value="XS" required />
</label>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you look up "Should the text associated with a radio button be placed on left or right of the radio button?"

Change is optional.

Comment thread Form-Controls/index.html
</div>
</div>

<br />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider using CSS to control spacing between elements instead of using <br> tags. This approach is generally more maintainable and semantically correct.

Comment thread Form-Controls/index.html

<label
>S:
<input type="radio" name="Size" value="S" required />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The convention is to use lowercase kebab-case values for the name attribute (and also for id and class attributes)

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants