Flexbox solution fails unit tests - tech docs project

[freecodecamp]

Flexbox solution fails unit tests - tech docs project

[tchaffee]

@QuincyLarson Before going ahead with creating a PR for this issue I would like to better understand the purpose of the "technical-documentation-page" assignment. Looking at the existing tests, I am guessing the goal for the assignment is to have the student use the <nav> (and related) elements along with some other basic elements like <code>, <li>, and <p>.

Concerning the layout, the <nav> element must stay visible, and the page should use at least one media query.

If the above is correct and I am not missing anything, then I recommend the following approach to the 2 issues described by @sue888.

The header requirement

"The first element within the navbar should be a <header> which contains text that describes the topic."

This can be improved by making the requirement slightly more flexible:

"The navbar element should contain one <header> element which contains text that describes the topic".

The existing test checks for firstElementChild which does not allow any container <divs> because the <header> must be the very first and top child. I would change this to use getElementsByTagName('header').length === 1. getElementsByTagName() searches the entire sub-tree so this would allow for any number of containers before the <header>. I think this test alone would be good enough, but if we wanted to make sure the header appears before any navbar links, I could try to add a test for that too. Something like "The <header> element inside the navbar must appear before any <a> elements with the class of "nav-link"."

NOTE: The codepen provided by @sue888 uses the <header> element for every link within the navbar, so my new proposed test would still cause that code to fail. However, I believe it should fail because using the <header> element for each <a> link in the navbar is not recommended. The <header> element should only be used once within the navbar.

The layout requirement

"On regular sized devices (laptops, desktops), the element with id="navbar" should be shown on the left side of the screen and should always be visible to the user."
failed with "AssertionError: expected -1000 to be close to 0 +/- 10

I believe this is working as intended. If you look at the working example pen, the element with id="navbar" has CSS specifying position:fixed, keeping the <nav> element on the top left.

But on the CodePen provided by @sue888, the element with id="navbar" does not have a fixed position.

The navbar appears to stay at a fixed position because a container within the navbar (<div id="fixed-or-not">) does have a fixed position. Visually all of the navbar content stays on the top left, but in reality the <nav> element actually does scroll off the top of the viewport when the user scrolls down.

I have not tried it, and I think it would be out of scope for me, but I believe it would be possible to make the navbar a fixed position while still using a flexbox layout. So I don't see a problem with keeping this test as-is, and having students make sure the <nav> element itself stays on the top left. I.e. this test is working as intended and the CodePen provided by @sue888 should be fixed instead.

Your thoughts?

[QuincyLarson]

@tchaffee Awesome! OK - It sounds like we can close this issue, and @sue888 can update her pen when she has time.

I'm going to close this issue but if there's anything else related to this, feel free to reopen it.

[no-stack-dub-sack]

@sue888 it's a good point, but we've been struggling to account for everything with these. Hard to be completely agnostic with the way people implement these projects when it comes to testing.

@QuincyLarson what do you think? We've been running in to plenty of similar issues that in one way or another limit people, finding it tough to keep up. "Perfect" may be more unattainable than usual for these...

[QuincyLarson]

@tchaffee I agree with you about

"The navbar element should contain one

element which contains text that describes the topic".

The existing test checks for firstElementChild which does not allow any container because the

must be the very first and top child. I would change this to use getElementsByTagName('header').length === 1. getElementsByTagName() searches the entire sub-tree so this would allow for any number of containers before the . I think this test alone would be good enough, but if we wanted to make sure the header appears before any navbar links, I could try to add a test for that too. Something like "The element inside the navbar must appear before any elements with the class of "nav-link"."

NOTE: The codepen provided by @sue888 uses the

element for every link within the navbar, so my new proposed test would still cause that code to fail. However, I believe it should fail because using the element for each link in the navbar is not recommended. The element should only be used once within the navbar.

I agree with you.

Please use this Pen (on freecodecamp's CodePen account) instead - we can refactor it to get it to pass your new tests. You can fork it then let me know when it passes and I can fork it back with the official account. https://codepen.io/freeCodeCamp/pen/NdrKKL

Here are all the freeCodeCamp pens for your reference: https://codepen.io/freeCodeCamp/

[tchaffee]

@QuincyLarson No need to refactor the example code. My new test works with the old code at the same time allowing more flexibility with multiple levels of container <div>. The only thing that will continue to break is the pen provided by @sue888. That code would need to be refactored, because the tests are correct and there are a couple of problems with the code provided by @sue888.

[tchaffee]

I still have a PR for this that I need to push when it is ready.

[no-stack-dub-sack]

@tchaffee this thorough explanation was helpful for me to better understand your PR. Everything looks good to me except the merge conflicts need to be resolved because @tbushman's PR was merged ahead of yours.

You can rebase on top of this and update the PR to fix. Then we should be good to go on this. I agree with your assessment and choices. Nice work!