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

Learn Typography by building a Nutrition Label:Step 22 #47109

Closed
MahijithMenon opened this issue Jul 31, 2022 · 20 comments · Fixed by #47370
Closed

Learn Typography by building a Nutrition Label:Step 22 #47109

MahijithMenon opened this issue Jul 31, 2022 · 20 comments · Fixed by #47370
Labels
first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. type: bug Issues that need priority attention. Platform, Curriculum tests (if broken completely), etc.

Comments

@MahijithMenon
Copy link

MahijithMenon commented Jul 31, 2022

Describe the Issue

In this ,we have to add span tag in between p tags and if we give spacing between text present in p tags only then the test cases is accepted.
This case will work:-

    //<div class="label">
      //<h1 class="bold">Nutrition Facts</h1>
      //<div class="divider"></div>
      //<p>8 servings per container</p>
      //<p class="bold">Serving size <span class="right">2/3 cup (55g)</span></p>
    //</div>

But This will not work:-

    // <div class="label">
       //<h1 class="bold">Nutrition Facts</h1>
       //<div class="divider"></div>
       //<p>8 servings per container</p>
       //<p class="bold">Serving size<span class="right"> 2/3 cup (55g)</span></p>
     //</div>

we can add a new test case which could allow the second one too or can add a description regarding it.

Affected Page

https://www.freecodecamp.org/learn/2022/responsive-web-design/learn-typography-by-building-a-nutrition-label/step-22

Your code

 //<div class="label">
      // <h1 class="bold">Nutrition Facts</h1>
       //<div class="divider"></div>
       //<p>8 servings per container</p>
       //<p class="bold">Serving size<span class="right"> 2/3 cup (55g)</span></p>
     //</div>

Expected behavior

This should have been worked or we could actually tell the user to rectify the spacing

Screenshots

No response

System

  • Device: [e.g. iPhone 6, Laptop]
  • OS: [e.g. iOS 14, Windows 10, Ubuntu 20.04]
  • Browser: [e.g. Chrome, Safari]
  • Version: [e.g. 22]
    Laptop
    Windows 11
    Brave

Additional context

No response

@MahijithMenon MahijithMenon added scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. status: waiting triage This issue needs help from moderators and users to reproduce and confirm its validity and fix. type: bug Issues that need priority attention. Platform, Curriculum tests (if broken completely), etc. labels Jul 31, 2022
@MahijithMenon
Copy link
Author

how to add html code it is showing the executed code

@Sboonny
Copy link
Member

Sboonny commented Jul 31, 2022

@MahijithMenon I have update your post, to show html

@Sboonny
Copy link
Member

Sboonny commented Jul 31, 2022

In step https://www.freecodecamp.org/learn/2022/responsive-web-design/learn-html-by-building-a-cat-photo-app/step-24 has similar instructions, because campers do face this earlier, I don't think we should change the description.

@hanswang123456
Copy link
Contributor

@MahijithMenon, it would be good to close the issue if no PRs are needed.

@Sboonny
Copy link
Member

Sboonny commented Aug 22, 2022

@hanswang123456 I disagree

@moT01
Copy link
Member

moT01 commented Aug 23, 2022

I don't know how much of a problem there is here. Using the code that will not pass above (<p class="bold">Serving size<span class="right"> 2/3 cup (55g)</span></p>), you get the hint:

Your span element should have the text 2/3 cup (55g).

So that should let users know to remove the space. If you then remove the space to use this code: <p class="bold">Serving size<span class="right">2/3 cup (55g)</span></p> - the hint is:

Your p element should still have the text Serving size 2/3 cup (55g).

Which should let them know that they need a space.

A quick forum search shows a few people adding that space, but many others that do it wrong - like including size in the span or something. I would be fine with making the tests a little more lenient on that space if we want.

@moT01 moT01 added status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. and removed status: waiting triage This issue needs help from moderators and users to reproduce and confirm its validity and fix. labels Aug 23, 2022
@hanswang123456
Copy link
Contributor

@moT01, removing the leading/trailing white space from the span assertion could be an option. That way, the test only has to confirm the correct text(leading/trailing space) and let the general p tag test handle any spacing issues.

@moT01
Copy link
Member

moT01 commented Aug 23, 2022

Alright, I'm going to open this up for help. We want to change the test so it allows a space at the start of that span, but keep the test for the paragraph so the whole sentence is correct. It's the third test in this file

@moT01 moT01 added first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. and removed status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. labels Aug 23, 2022
@Xavier-Pierre-dev
Copy link
Contributor

Xavier-Pierre-dev commented Aug 23, 2022

I don't know how much of a problem there is here. Using the code that will not pass above (<p class="bold">Serving size<span class="right"> 2/3 cup (55g)</span></p>), you get the hint:

Your span element should have the text 2/3 cup (55g).

So that should let users know to remove the space. If you then remove the space to use this code: <p class="bold">Serving size<span class="right">2/3 cup (55g)</span></p> - the hint is:

Your p element should still have the text Serving size 2/3 cup (55g).

Which should let them know that they need a space.

A quick forum search shows a few people adding that space, but many others that do it wrong - like including size in the span or something. I would be fine with making the tests a little more lenient on that space if we want.

When you are describing like this it's make sens but inside the hint for the test say :
Your span element should have the text 2/3 cup (55g).

But people can misinterpret, thinking that the span element must contain the text and not the exact text without space. After all <span class="right"> 2/3 cup (55g) </span> contains 2/3 cup (55g).


I think the test could be change from :

assert(document.querySelector('span')?.textContent === '2/3 cup (55g)');

to

assert(document.querySelector('span')?.textContent.includes('2/3 cup (55g)'));

I add an image to illustrate :
illustrate

code snippet
    <div class="label">
      <h1 class="bold">Nutrition Facts</h1>
      <div class="divider"></div>
      <p>8 servings per container</p>
      <p class="bold">Serving size <span> 2/3 cup (55g)  <span></p>
    </div>
    <script>
      
      if(document.querySelector('span')?.textContent.includes('2/3 cup (55g)'))
      {
          document.querySelector("p").style.backgroundColor = "red";
      }
    </script>

@hanswang123456
Copy link
Contributor

hanswang123456 commented Aug 23, 2022

@Xavier-Pierre-dev, it might be better to remove spaces then check for exact occurrence of '2/3 cup (55g)' . This way if users insert unwanted text, it won't pass the span's assertion.(this is mostly for error messages so users know to target the span tag)

Maybe something like:
assert(document.querySelector('span')?.textContent.trimStart() === '2/3 cup (55g)'); to remove whitespaces at the start.

And then do assert(document.querySelectorAll('p')?.[1]?.textContent === 'Serving size 2/3 cup (55g)'); so you have the correct textContent inside the p tag.

@Xavier-Pierre-dev
Copy link
Contributor

Xavier-Pierre-dev commented Aug 23, 2022

I'm not sur to understand your point if you remove all the space then 2/3cup(55g) will also pass.
illustrategif

In fact with includes i already check if the text contains 2/3 cup (55g) so if something is added inside this chain of string that's will not pass, howewer if something is added outside that's will still pass because the span element still contain 2/3 cup (55g).

I think this behaviour follow the hint sentence for example 2/3 cup (55g) something should also pass since the hint ask :
Your span element should have the text 2/3 cup (55g).

and 2/3 cup (55g) something contain 2/3 cup (55g)

@hanswang123456
Copy link
Contributor

Includes doesn't check for letters added to the start or end which can make error messages less specific.

@moT01
Copy link
Member

moT01 commented Aug 23, 2022

One issue I saw when I looked on the forum was people doing something like this: <p class="bold">Serving <span class="right">size 2/3 cup (55g)</span></p> (they put size in the span, not supposed to)

So using .includes would allow that to pass. I don't think we should be that lenient. Extra spaces, sure - extra text, I don't like it. I am fine with removing the whitespace before (and after?) the text content. Either way works for me.

@hanswang123456
Copy link
Contributor

hanswang123456 commented Aug 23, 2022

@moT01 for the assertion on the p tag: assert(document.querySelectorAll('p')?.[1]?.innerText=== 'Serving size 2/3 cup (55g)'), it seems like textContent is more specific than innerText and forces users to only have 1 space.

@Xavier-Pierre-dev
Copy link
Contributor

Xavier-Pierre-dev commented Aug 23, 2022

yes exactly but I think this is exactly what the hint ask here
Your span element should have the text 2/3 cup (55g).

and 2/3 cup (55g) something contain 2/3 cup (55g)

otherwise the hint should be changed to :
Your span element should only contain 2/3 cup (55g).

in this case i think I think regex will be better :
illustrategif3

code snippet
    <div class="label">
      <h1 class="bold">Nutrition Facts</h1>
      <div class="divider"></div>
      <p>8 servings per container</p>
      <p class="bold">Serving size <span> 2/3 cup (55g)<span></p>
    </div>
    <script>
      if(document.querySelector('span')?.textContent.match(/^(\s+|)2\/3 cup \(55g\)(\s+|)$/))
      {
          document.querySelector("p").style.backgroundColor = "red";
      }
    </script>

edit : I update the comment to put the regex so that white space are allowed in start or end but if something else is added then it's will fail

@hanswang123456
Copy link
Contributor

hanswang123456 commented Aug 23, 2022

@Xavier-Pierre-dev, regexes do work. The issue is that users sometimes put whitespaces inside the span tag. For the regex, include 1 optional whitespace at the start of the string so there is more flexibility for users. I think that would work well in conjunction with the p tag assertion.

@Xavier-Pierre-dev
Copy link
Contributor

Xavier-Pierre-dev commented Aug 23, 2022

so here difference beetween the regex is that we can easily allow white space before and after the string so it's more near to
Your span element should only contain 2/3 cup (55g).

but at the same time the hint say :
Your span element should have the text 2/3 cup (55g).
and in this case I think the example I provide with include should work because 2/3 cup (55g) something contains 2/3 cup (55g) so that's already follow the hint provided and link to this test.

I'm not sur if one solution is better than the other I think both will work

  • allow multiple white space before and after 2/3 cup (55g) :
document.querySelector('span')?.textContent.match(/^(\s+|)2\/3 cup \(55g\)(\s+|)$/)

  • allow only one white space before and after 2/3 cup (55g) :
document.querySelector('span')?.textContent.match(/^(\s|)2\/3 cup \(55g\)(\s|)$/)

  • allow only one white space before 2/3 cup (55g) : (maybe this is the right thing to do since the concern is that some people add one space before and some people didn't put a space before)
document.querySelector('span')?.textContent.match(/^(\s|)2\/3 cup \(55g\)$/)

  • allow content before and after 2/3 cup (55g) :
document.querySelector('span')?.textContent.includes('2/3 cup (55g)')

@hanswang123456
Copy link
Contributor

I think only allowing whitespaces before the element works best. So something like your second version. document.querySelector('span')?.textContent.match(/^(\s|)2\/3 cup \(55g\)$/)

@Xavier-Pierre-dev
Copy link
Contributor

I think only allowing whitespaces before the element works best. So something like your second version. document.querySelector('span')?.textContent.match(/^(\s|)2\/3 cup \(55g\)$/)

yes I think also I was editing my comment at the same time to add this.

@hanswang123456
Copy link
Contributor

Feel free to open a PR and reviewers can review the code.

Sembauke pushed a commit that referenced this issue Aug 24, 2022
* fix/Learn Typography by building a Nutrition Label:Step 22 #47109

* revert change on non-English files

* Update curriculum/challenges/english/14-responsive-web-design-22/learn-typography-by-building-a-nutrition-label/615f42a021625f656101ef93.md

Co-authored-by: Naomi Carrigan <nhcarrigan@gmail.com>

Co-authored-by: Naomi Carrigan <nhcarrigan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. type: bug Issues that need priority attention. Platform, Curriculum tests (if broken completely), etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants