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 NORTH WEST| HABEN SOLOMON KIDANE| Module User Focused Data| Form controls| week 2 #298

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

Conversation

Habenk-coder
Copy link

@Habenk-coder Habenk-coder commented Oct 28, 2024

CYF NORTH WEST | HABEN SOLOMON KIDANE | Module User Focused Data| Form Controls | WEEK2

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 COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Needs Review

Copy link

netlify bot commented Oct 28, 2024

Deploy Preview for cyf-ufd ready!

Name Link
🔨 Latest commit 7568b43
🔍 Latest deploy log https://app.netlify.com/sites/cyf-ufd/deploys/671fec11169aaf0008fc69a0
😎 Deploy Preview https://deploy-preview-298--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.

@Habenk-coder
Copy link
Author

I am looking for your constructive feedbacks

@Habenk-coder Habenk-coder changed the title Form controls CYF NORTH WEST| HABEN SOLOMON KIDANE| Module User Focused Data| Form controls| week 2 Oct 28, 2024
@Habenk-coder Habenk-coder marked this pull request as draft October 28, 2024 20:22
@Habenk-coder Habenk-coder marked this pull request as ready for review October 28, 2024 20:35
@Grajales-K Grajales-K added the Needs Review Participant to add when requesting review label Oct 31, 2024
@cjyuan cjyuan added the 👀 Review Git Changes requested to do with Git label Nov 2, 2024
@cjyuan
Copy link

cjyuan commented Nov 2, 2024

  • The current branch is created from wireframe branch and not from the main branch. Please try to seek help from volunteers in Saturday workshop to rebase this branch so that it branch out from main.

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

@SallyMcGrath SallyMcGrath left a comment

Choose a reason for hiding this comment

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

Thanks for this @Habenk-coder . There are a few things going on in here for you to look at.

First, please back out your other changes and just open a PR with the changes related to form controls. Next, please review the acceptance criteria and check your submission meets them. It's helpful to write them out one by one as comments in your file to help you check this.

Make sure to run Lighthouse and follow the advice it gives you to make your changes. Good luck! Come to class to get help if you are stuck.

voluptatem accusantium, necessitatibus cum accusamus, id cumque harum
voluptates ipsum nisi vero illo dolore! Doloribus, quae hic. Facilis
voluptate nobis eum.
we believe that every celebration deserves a cake that is as unique and special as the occasion itself.
Copy link
Member

Choose a reason for hiding this comment

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

It looks like there are files in this PR that don't belong to this project. Can you open a PR with just the changes for Form Controls, please? Come to class to get help with this if you are stuck.

<h2>T-shirt Order Form</h2>
<form name="tshirtForm">
<label for="name">Customer's Name:</label>
<input type="text" id="name" name="name" required minlength="2" pattern="[A-Za-z]+">
Copy link
Member

Choose a reason for hiding this comment

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


<label for="email">Customer's Email:</label>
<input type="email" id="email" name="email" required>
<div class="error-message">Please enter a valid email address.</div>
Copy link
Member

Choose a reason for hiding this comment

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

What will happen with these error messages? Can you talk through what you expect?

</div>

<script>
document.addEventListener("DOMContentLoaded", function() {
Copy link
Member

Choose a reason for hiding this comment

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

Does this meet the criteria listed in the readme? Can you explain how it does, or what your goal in with including this?

text-decoration: none;
}

footer a:hover {
Copy link
Member

Choose a reason for hiding this comment

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

What would happen if I wasn't using a mouse?

@SallyMcGrath SallyMcGrath added the 👀 Review Requirements Changes requested to meet requirements label Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 Review Git Changes requested to do with Git 👀 Review Requirements Changes requested to meet requirements Reviewed Volunteer to add when completing a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants