Skip to content
This repository was archived by the owner on Oct 2, 2024. It is now read-only.

NW5-Manchester-Zaw Khing-HTML-CSS-challenges#222

Open
zkhing wants to merge 4 commits into
CodeYourFuture:mainfrom
zkhing:main
Open

NW5-Manchester-Zaw Khing-HTML-CSS-challenges#222
zkhing wants to merge 4 commits into
CodeYourFuture:mainfrom
zkhing:main

Conversation

@zkhing
Copy link
Copy Markdown

@zkhing zkhing commented Jan 13, 2023

No description provided.

Copy link
Copy Markdown
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, @zkhing ! I really appreciate the effort you've put in here.

Thinking a bit more strategically about using this exercise to develop your own learning, how might you go back now and revise your code?

Some things to consider:

  1. Are you happy with this design? Would you put this in your professional portfolio? If not, why not, and what will you do about it?
  2. What would you like to learn more about, of the things discussed in this review? What is your plan to develop that expertise?
  3. What did you learn from this exercise already, and what part of your code are you particularly pleased with? (I thought your coverage score was impressive!)

Comment thread Form-Controls/index.html
<form>
<!-- write your html here-->
<!-- try writing out the requirements first-->
<div>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there another sectioning element specifically for forms?

How else might you group a set of fields inside a form, and what is the effect of using a different element?

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

</html> No newline at end of file
<div>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there another element you could have chosen to use instead? Why does it matter how we group form controls?

https://www.w3.org/WAI/tutorials/forms/grouping/

Comment thread Form-Controls/index.html Outdated
<option value="m">M</option>
<option value="l">L</option>
<option value="xl">XL</option>
<option value="xxl">XL</option></select
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Psst, here, the value is xxl but the option text says XL

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for your review Sally. I know it is silly mistake and I was rushing a bit :) I will take extra care in future.

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

<input type="submit" value="Submit" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sometimes people use button and sometimes people use input submit. What is the difference and how do you decide which to use?

Comment thread Form-Controls/index.html
value="red"
checked
/>
<label for="red">Red</label><br />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some things to consider:

  1. Is a br tag a good idea here?
  2. Do we ever use html for layout only?
  3. In our discussions with Lucy, what has she said about br tags?

Comment thread Form-Controls/styles.css Outdated
h2{
text-align: center;
color: #354f52;
font-size: 12px;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(Don't use font sizes less than 16px for most things.)

Are pixel sizes the best choice for fonts? I think this lesson would be good for you to explore

https://web.dev/learn/design/typography/

Comment thread Form-Controls/index.html
id="name"
name="name"
pattern="[a-zA-Z\s]+"
minlength="2"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

Comment thread Form-Controls/index.html

<footer>
<!-- change to your name-->
<h2>Created by Zaw Khing</h2>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

💯

Comment thread Form-Controls/styles.css

.product-form{
background-color: #588157;
margin: 0px 10em;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How does em sizing function? What would happen if a user was using this page at 200% text size?

Comment thread Form-Controls/styles.css Outdated
padding: 10px 20px;
}

#size{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you need an id here? How did you decide to use an id? Thinking through this decision carefully is useful. You might like this article to help you deliberate. https://csswizardry.com/2011/09/when-using-ids-can-be-a-pain-in-the-class/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants