Closed yoizfefisch closed 6 years ago
@yoizfefisch
Thanks for taking the time to look into this and identifying the challenges, do you have some opinion on what should be a good fix/update?
IMHO, the goal is teaching the concepts first, and recommending best practices, where possible. Basically the idea is to be succinct and not overwhelming the users, with a overload of information.
We can always, refer to more informative and accurate resources like MDN, or even linking to the dedicated challenge in this case for example.
We would like to hear some opinion before throwing this open for contributions.
Just a thought... What if you added extra points following best practices? something like adding optional test cases or something. Just thinking out loud here
Hi @davinderpalrehal
Just a thought... What if you added extra points following best practices? something like adding optional test cases or something. Just thinking out loud here
That is super brilliant thought, kudos for thinking out loud! 🎉 👏
Do you mind creating a fresh issue for this, this is simple to implement, I think. But, will have to be deferred until after beta goes out.
Thanks again for the suggestion.
/cc @QuincyLarson
@raisedadead this is gonna be my first time contributing 😃 . Any suggestions for the name of the issue?
How about something on the lines of :
Teaching best-practices by awarding bonus brownie points for optional tests.
Do, remember to add the details, and maybe a link to this issue should be helpful.
@raisedadead To solve the issue at hand, I would suggest the following 2 options:
Update the lesson to include the following:
<label for="indoor">
<input type="radio" name="indoor-outdoor" id="indoor">Indoor
</label>
It is considered best practice to add a
for
attribute to thelabel
that matches theid
attribute of the child element. This helps assistive technologies link the relationship between thelabel
and its child elements, such as aninput
in this case.
Create a separate lesson for the label
element after the input
types are covered (BTW, should we not have one for textarea
?).
In this lesson it would make sense to discuss the benefit of nesting the input
element in a label
element; that it makes the text of the label clickable.
It could then be discussed that another way of achieving this is setting the for
and id
attributes.
Then, the above snippet could be added pointing out that even using the nested method, best practice is to still set the for
and id
attributes.
I can see a slight challenge with these solutions since the HTML section doesn't mention the id
attribute at all.
I also spotted this issue while going through the beta curriculum; yoizfefisch mentioned he had raised it here, so I hope it is ok to add my comment to the discussion.
I'd second his suggestions above, that a separate lesson for label
be created after the input types are introduced.
I think that introducing the id
element in a lesson prior to that for label
shouldn't present a major issue in terms of trying to separate HTML and CSS into discrete sections, as long as the lesson explains that it will be expanded upon in the CSS lessons perhaps?
@raisedadead @davinderpalrehal I am wary of adding "optional" tests or bonus tests, as they introduce further ambiguity into the challenge. People will think: "do I need to do these?" and skip them, but then have a feel anxious about it later.
Instead, I think we need to just decide: do we need an extra challenge? If so, where should that challenge be located?
@raisedadead is right - we don't want to be pedantic and overload campers with best practices - it's better if we can push those back farther into the curriculum, so they can first focus on learning the basics and building projects.
But we may be able to redesign some of these challenges, as @SilentGamelan suggested, to either not use antipatterns at all, or to quickly cover a topic, like labels, before moving on.
What do you all recommend we do here?
@QuincyLarson I get your point for having optional tests, and also agree @raisedadead about overloading the campers.
I think the id
attribute has been introduced before while doing the bootstrap module maybe just explain the different functions of the id
with regards to labels.
@QuincyLarson @raisedadead I think we can keep it simple by removing the for
attribute from the radio button lesson. As I mentioned in my first comment, the label
element with its for
attribute are covered later in the applied accessibility section.
In the radio button lesson, add a few words to explain that wrapping the input
element inside the label
element makes the label's text clickable.
Later, in the label lesson or the fieldset lesson, add a refernce to the former lesson, something along the lines of:
You learned about radio buttons and its labels in a previous lesson. In that lesson we wrapped the radio button's
input
element inside alabel
element along with the label's text in order to make it clickable. Another way to achieve this is by using thefor
attribute as explained in this lesson.
@SilentGamelan @davinderpalrehal What do you think? Does the for
attribute add anything to this lesson or is it just confusing the reader?
@yoizfefisch it probably adds to the usability part of things but might be something that campers can learn more of once they have mastered the basics when they start thinking of user experience. I mean the only thing the for
attribute really adds is that if a user clicks on the label the field associated to it is highlighted or selected.
Yeah, I'd agree with the above @yoizfefisch and @davinderpalrehal .
I think it would streamline the HTML section to move the for
element out of its current place, which would also then avoid having to crowbar in an explanation of the id
tag.
I don't think it adds much to the lesson as is, when it can just be covered soon after in the greater context of applied accessibility - I'd be inclined to move it as the simplest solution.
@SilentGamelan @yoizfefisch @davinderpalrehal Great - it sounds like we've all reached a consensus on what needs to be done here. Which one of you would be interested in updating this challenge to remove its use of for
?
I would love to work on this.
@davinderpalrehal Awesome! It's all you then. Please let me know if I can do anything to help. Looking forward to seeing your updated challenge(s).
This is awesome, @QuincyLarson will touch base with you on gitter later on should be starting work on this tonight. Thanks guys really looking forward to this.
I just got a message from @davinderpalrehal that he won't be able to resolve this issue. Is anyone else interested in helping us with it?
@QuincyLarson I'd love to take a crack at this one!
@elbaumpj Great! It's yours! Thanks for your help! Please keep us posted on your progress with this :)
Challenge Name
Basic HTML and HTML5: Create a Text Field Basic HTML and HTML5: Create a Set of Radio Buttons Basic HTML and HTML5: Create a Set of Checkboxes
Issue Description
This is a follow up to issue #15946 where it was suggested that the lesson Basic HTML and HTML5: Create a Set of Radio Buttons should reflect the best practice guidelines from the Mozilla documentation.
The issue was closed and now the lesson includes the following:
Yet:
label
element'sfor
value has to match theinput
element'sid
value as explained in the linked Mozilla documentation page.input
elements, the Mozilla page actually discussesinput type="text"
which is covered in a previous lesson: Basic HTML and HTML5: Create a Text Field.