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

CYF-ITP - South Africa |Azharuddin Meyer | Module-User Focused Data-Form Controls | WEEK2 | #320

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

Conversation

azharuddinmeyer
Copy link

COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK

Learners, PR Template

Self checklist

  • [ yes] I have committed my files one by one, on purpose, and for a reason
  • [ yes] I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • [ yes] I have tested my changes
  • [ yes] My changes follow the style guide
  • [ yes] My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

@azharuddinmeyer azharuddinmeyer added the Needs Review Participant to add when requesting review label Nov 10, 2024
Copy link

netlify bot commented Nov 10, 2024

Deploy Preview for cyf-ufd ready!

Name Link
🔨 Latest commit 11729f7
🔍 Latest deploy log https://app.netlify.com/sites/cyf-ufd/deploys/673d8430a78c8400088d968c
😎 Deploy Preview https://deploy-preview-320--cyf-ufd.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link

@jamesbaskerville jamesbaskerville left a comment

Choose a reason for hiding this comment

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

Really nice start @azharuddinmeyer, just a few comments on requirements!

Comment on lines 31 to 37
<input
type="text"
name="name"
id="name"
placeholder="Username"
required
/>

Choose a reason for hiding this comment

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

What validations might we want to check for the name field?

Copy link
Author

Choose a reason for hiding this comment

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

Hi james, I added minlength and maxlength validations for my name field

Comment on lines 77 to 78
max="2024-11-16"

Choose a reason for hiding this comment

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

nice validations! however, the form currently allows me to submit without filling out this field. Could you fix that?

Copy link
Author

Choose a reason for hiding this comment

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

Added a required field for my date field

@jamesbaskerville jamesbaskerville added 👀 Review Requirements Changes requested to meet requirements Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Nov 10, 2024
@azharuddinmeyer azharuddinmeyer added Needs Review Participant to add when requesting review and removed 👀 Review Requirements Changes requested to meet requirements Reviewed Volunteer to add when completing a review labels Nov 10, 2024
@azharuddinmeyer azharuddinmeyer changed the title CYF-ITP - South Africa |Azharuddin Meyer | Module-User-Focused-Data Form Controls | Week2 CYF-ITP - South Africa |Azharuddin Meyer | Module-User-Focused-Data Form Controls | WEEK2 | Nov 10, 2024
@azharuddinmeyer azharuddinmeyer changed the title CYF-ITP - South Africa |Azharuddin Meyer | Module-User-Focused-Data Form Controls | WEEK2 | CYF-ITP - South Africa |Azharuddin Meyer | Module-User Focused Data-Form Controls | WEEK2 | Nov 10, 2024
@cjyuan
Copy link

cjyuan commented Nov 12, 2024

Form meets all acceptance criteria. HTML code has some syntax error.

May I suggest using a HTML validator (https://validator.w3.org/) to check for potential syntax errors in your HTML code?

As to further limit what value can go into the name input textfield, you can consider using the pattern attribute.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Nov 12, 2024
@azharuddinmeyer azharuddinmeyer added Needs Review Participant to add when requesting review and removed Reviewed Volunteer to add when completing a review labels Nov 12, 2024
@KudahShambare
Copy link

KudahShambare commented Nov 13, 2024

Screenshot (19)

Good work !! You just have some issues in your CSS Code

You can use this validator

@KudahShambare KudahShambare added the Reviewed Volunteer to add when completing a review label Nov 13, 2024
@azharuddinmeyer azharuddinmeyer removed the Reviewed Volunteer to add when completing a review label Nov 16, 2024
</footer>
</body>
<body>
<div class="container">

Choose a reason for hiding this comment

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

Hi Azharuddin, is there another element that can be used for semantic reasons?
We also want to think about accessibility when it comes to using elements.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Ryno, I fixed my footer using an address element.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review Participant to add when requesting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants