Skip to content

London | May-2025 | Gideon Defar | form control #726

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

Closed

Conversation

gideondefar
Copy link

@gideondefar gideondefar commented Jun 12, 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

Changelist

Added form validation for customer name to prevent empty or whitespace-only inputs.

Ensured email addresses follow a valid format using HTML5 input type.

Limited t-shirt color selection to 3 predefined options (Red, Blue, Black).

Restricted t-shirt size selection to 6 fixed sizes (XS, S, M, L, XL, XXL).

I have used HTML only. I have not used any CSS or JavaScript.

Questions

Ask any questions you have for your reviewer.

Copy link

netlify bot commented Jun 12, 2025

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 956c48d
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/684c383cdfb5550008549042
😎 Deploy Preview https://deploy-preview-726--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.

@gideondefar gideondefar changed the title London | May-2025 | Gideon Defar | from control London | May-2025 | Gideon Defar | form control Jun 12, 2025
@gideondefar gideondefar added the Needs Review Participant to add when requesting review label Jun 13, 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.

Code is free of error and implementation is pretty solid!

  1. Could you improve the Lighthouse Accessibility score (as reported by the browser's Lighthouse dev tool) to 100?

  2. Could you update the brief description in the "Changelist" section to clearly summarize what this PR is about?
    image

  3. A PR branch should include only the changes relevant to the specific task or feature it addresses. You can see all the modified files in the current branch in the "Files changed" tab of this PR:
    https://github.com/CodeYourFuture/Module-Onboarding/pull/726/files

As the files in the "Wireframe" folder are not related to the Form-Control exercise, can you revert the changes made in those files in order to make this branch clean?

Note: It looks like this branch was created from your Wireframe branch instead of from main, which has caused changes from the Wireframe branch to appear here as well.

To revert the changes made to certain files, you can replace them with the versions from your main branch
(The files in the main branch remain unmodified since you forked them from CYF.)
Here’s how to do it:
- Download the original versions of the files from your main branch on GitHub.
- On your computer (in VSCode), switch to the branch where you want to revert the changes. (feature/from-control)
- Replace the modified files with the downloaded versions from the main branch.
- Commit the changes and push the commit to GitHub.

Comment on lines 20 to 27
<input
type="text"
id="name"
name="name"
required
minlength="2"
placeholder="Enter your full name"
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently a user can enter a name consisting of only space characters (e.g., " "). Could you enforce a stricter validation rule using the pattern attribute to disallow any name that contains only space characters?

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

cjyuan commented Jun 13, 2025

Here is an alternate approach you can try to fix your branch:

This instructions assume you had created a branch named B2 from a branch named B1 instead of from main, and you wanted to rebase B2 from B1 to main.

In your case,

  • B2 is feature/from-control (you misspelled "form" in the branch name).
  • B1 is feature/wireframe

0. Open the cloned "Module-Onboarding" repo in VSCode

1. Open a terminal in VSCode

2. Switch to the branch you want to rebase (B2)

git switch B2

Note: You can check which branch is the current branch via the command git branch (to list all branches with current branch highlighted)

3. Rebase B2 from B1 onto main

git rebase --onto main B1 B2

For more details about this command, see
https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase#:~:text=interactive%20rebase%20display-,Advanced%20rebase%20application,-The%20command%20line

4. Update (and Overwrite) your files in branch B2 at Github

While you are in branch B2 and you have verified that it has been successfully rebased, execute the following command:
git push --force origin

@gideondefar gideondefar force-pushed the feature/from-control branch from b3af46b to d845fd4 Compare June 13, 2025 12:16
@gideondefar
Copy link
Author

gideondefar commented Jun 13, 2025 via email

@cjyuan cjyuan added Complete Volunteer to add when work is complete and review comments have been addressed and removed Reviewed Volunteer to add when completing a review labels Jun 13, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Jun 13, 2025

Changes look good. All issues (except fixing the branch) have been addressed.

You should still try fixing this branch.

@gideondefar gideondefar deleted the feature/from-control branch June 13, 2025 19:04
@gideondefar
Copy link
Author

gideondefar commented Jun 13, 2025 via email

@cjyuan
Copy link
Contributor

cjyuan commented Jun 13, 2025

What do you mean by "one more time"?

@gideondefar
Copy link
Author

gideondefar commented Jun 13, 2025 via email

@cjyuan
Copy link
Contributor

cjyuan commented Jun 13, 2025

You can ask a volunteer on Saturday in an in-person workshop to help you resolve the issue.

Your branch issue won't affect the "Complete" status on this PR.

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