Skip to content

London|May-2025| Ping-Wang|Form controls #712

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

Conversation

pathywang
Copy link

@pathywang pathywang commented Jun 6, 2025

✅ 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

I forked form controls from CYF to my own repo then copied to my local vscode after create branch form-Controls ,did HTML code followed readme.md requirements then I did pull requests. During this time, I did commit a few times after review by CYF team. Today in order to clean my branch, I was trying to take off wireframe from branch form-Controls. unfortunately after I changed form-Controls branch name to form-controls ,my originally pull request has been closed so I have to get new pull creast then create pull request which I have done all on my own with ChatGpt help.

Copy link

netlify bot commented Jun 6, 2025

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 2eebf1b
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/68436536c3ebff0008883a1f
😎 Deploy Preview https://deploy-preview-712--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: 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 project configuration.

Delete Wireframe/index.html from form-controls branch
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.

You didn't have to close and delete the previous PR.

Deleted file is still considered a change by Github.

Now that you have deleted Wireframe/index.html, can you use the version in main to replace it?

  1. On your computer, in VSCode, open the Module-Onboarding repo, and switch to the form-controls branch.

  2. Visit https://github.com/pathywang/Module-Onboarding/blob/main/Wireframe/index.html

That's the version of the file in your main branch on GitHub.

  1. Download the file into the "Wireframe" folder.

  2. Commit the change, and push the commit to Github.

@cjyuan
Copy link
Contributor

cjyuan commented Jun 6, 2025

Can you also update your PR description to change your checkboxes from
image

to
image


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.
    • 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.

@cjyuan cjyuan added the Reviewed Volunteer to add when completing a review label Jun 6, 2025
@pathywang
Copy link
Author

thank you for quick review, first i do not how how to tick on PR description, secondly I think in wireframe there was commit done, in my local vscode, there is no wireframe index,HTML but show on git hub, I did with chatgpt, through terminal commit but it does not work as you mentioned that go to the main. i can not understand there is no wireframe HTML under form-controls branch but it just show on github, I do not know why

@pathywang
Copy link
Author

there is no wireframe file under my form-contros branch in vscode but it does show on the git hub, I do not know why?

@cjyuan
Copy link
Contributor

cjyuan commented Jun 6, 2025

Can you find out from your "Wireframe" PR how you checked the checkboxes in the PR description, and then apply the same syntax in this PR? I will mark this PR as "Complete" if you can fix the checkboxes.

Regarding the branch, please seek assistance from a volunteer during a Saturday in-person workshop to resolve the problem. If you don't know what you did wrong, you may still face the same problem in future PRs.

@cjyuan
Copy link
Contributor

cjyuan commented Jun 6, 2025

I updated your PR description with some samples.

@pathywang
Copy link
Author

thanks, i ticked checkboxes now, hopefully it is ok with you. I will definitely ask volunteer about this PR issues

@pathywang pathywang added the Needs Review Participant to add when requesting review label Jun 6, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Jun 6, 2025

Your implementation of the form is solid. I will mark this PR as "Complete" first.

Please follow the instructions in #570 to prepare what you need to submit on the Course Platform.


  • The check boxes are still not properly checked.
  • Please find someone to help you to figure out how to fix your branch or why you can replace files based on my instructions.

@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 6, 2025
@pathywang
Copy link
Author

Thank you for your message, i will do as your instruction. i know I am struggling with lots of task and hopefully I will overcome them soon with CYF team help.

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