Getting initial to work at the top of the code?

Then I would need to start from this code.

This is the one where the sound works.

Let’s start testing…

Help me to understand what purpose this code serves.

Removed:


.title {
  display: block;
}

Added:

.title {
  display: block;
}

I see no visual difference, what exactly is its purpose?

A better question is, how come this code doesn’t require a display property?

And this one does?
I don’t even know what the reason was, why a display property was added to it to begin with.

If a div is a block element by default, why would someone add it to the CSS?

<div class="title"></div>

.inactive .title {
  display: block;
}

It seems to me to be completely unnecessary.
@PaulOB #182

I have 2 questions for you concerning your markup of the CSS.

The first question is:

What purpose was this meant to serve in the code, what’s it for?
.title{display:none}

The second question is:

What purpose was this meant to serve in the code, what’s it for?
.inactive .title{display:block;}

When I remove both codes from the CSS, I see no difference.

Are those codes needed, or necessary at all?

Wait a second:

<h1 class="title">Links</h1>

Is an inline-block; display property.

That’s why you had put display:block; on it.

If it was a div instead of h1, then display:block; can be omitted.

I think I have that right.

Now that would just leave my first question that’s remaining.

What purpose was this meant to serve in the code, what’s it for?
.title{display:none}

When I remove it from the CSS, I see no difference.

Is that needed, or necessary at all?

You can’t look at CSS rules in isolation. You need to look at the bigger picture.

You only want the “Links” text to display when the player is inactive, don’t you? You don’t want it there all the time. So adding display: none; prevents it showing.

But when it has the class of inactive, you do want the text to show, so you have to override the display: none; and set it back to its default state of display: block;

Removing both

.title{display:none}
.inactive .title{display:block;}

Would be wrong to do, even if it works without them?

What I mean is, are those codes necessary at all, if without them added, there’s no visual difference.

Firstly, as I’ve said before, PaulOB is not only more of an expert on CSS than I am, but he’s more of an expert on CSS than most folk (and not just folk on these forums), so if he gives you working code, you’d be wise to accept it, rather than try picking it apart to “improve” it.

That said, it looks as if .hidee is also being used to hide the intital image and text, so it may not be necessary to use both. I’m guessing Paul included both sets of rules to ensure it didn’t break if you made changes to one or other. I haven’t looked at it in detail, or tested it in other browsers, so I may have missed something.

Firstly, as I’ve said before, PaulOB is not only more of an expert on CSS than I am, but he’s more of an expert on CSS than most folk

In my initial question, I said:

I have 2 questions for you

I was waiting to hear from him because he made the code, so he would have a better understanding.

And you did explain for me the usage of these very well,
Now I have a better understanding of how those work.
And I thank you for that.

.title{display:none}
.inactive .title{display:block;}
1 Like

@PaulOB

Javascript is disabled here:

Since the cover is not showing, does that mean I need to fix it?

Is this better, is this what you mean?

I changed this:

<h1 class="title">Links</h1>

.title {
  height: 168px;
  margin: 0;
  background: url("https://i.imgur.com/BBYxKcf.jpg");
  border: 3px solid #0059dd;
  font-family: Tahoma;
  font-weight: bold;
  font-size: 30px;
  color: #0059dd;
  line-height: 100px;
  text-align: center;
  cursor: pointer;
}

To This:

<div class="title" title="OPEN">
    <p>Links</p>
  </div>

.title {
  width: 266px;
  height: 174px;
  background: url("https://i.imgur.com/BBYxKcf.jpg")no-repeat;
  cursor: pointer;
  border: 3px solid #0059dd;
  box-sizing: border-box;
}

.title p {
  font-family: Tahoma;
  font-weight: 900;
  font-size: 30px;
  color: #0059dd;
  line-height: 40px;
  text-align: center;
}

When I enable javascript:

Both these codes are no longer needed, if that’s correct?

.title{display:none;}
.inactive .title{display:block;}

No that means the opposite and the page is usable and defaults to a working page when Js is disabled.

If you removed those css rules then the cover would show and be of no use to any one

This was all explained at the start and you seem to be circling back and fiddling with things that were already tried and tested!

2 Likes

How can it be better? You changed something that worked into something that didn’t work just to save adding a couple of properties.

2 Likes

Oh, so with the javascript disabled, what I want to see are the links, not the cover image?

I think I get it now.


So, this would work too, then.
Just a different way of writing the code.

 <div class="title" title="OPEN">
    <p>Links</p>
  </div>

.title {
  width: 266px;
  height: 174px;
  background: url("https://i.imgur.com/BBYxKcf.jpg")no-repeat;
  cursor: pointer;
  border: 3px solid #0059dd;
  box-sizing: border-box;
}

.title p {
  font-family: Tahoma;
  font-weight: 900;
  font-size: 30px;
  color: #0059dd;
  line-height: 40px;
  text-align: center;
}

.title {
  display: none;
}
.inactive .title {
  display: block;
}

With this code, how come I’m not seeing the links with javascript disabled, and it’s just blank white space? @PaulOB

I’m perplexed by this.

This code has no typed text, it’s only an image.
Shouldn’t the links still be showing with the javascript disabled?
Like how the other code does it?

<div class="cover"></div>

.cover {
  width: 266px;
  height: 174px;
  background: url("https://i.imgur.com/MbJ5O2d.png") no-repeat;
  cursor: pointer;
  border: 3px solid #0059dd;
  box-sizing: border-box;
}

.cover {
  display: none;
}

.inactive .cover {
  display: block;
}

Let’s start indeed. After adding external resources and the test runner, the basic test shows that there’s a conflict between your CSS styles and the reporter. https://jsfiddle.net/pmw57/ffvkbLjw/304/

The title class is used by both of them, so updating your title class so that it is only applied inside of the .wrap class, and the testing is ready to begin. https://jsfiddle.net/pmw57/ffvkbLjw/305/

What test can we use that will accurately tell us that the problem with the links image is fixed? Looking at the console view of the code, I see that class names seem to be correctly added to it, so checking class names will not in this case be of an use.

The problem is that the links image is being displayed when it shouldn’t be displayed, so we can check its computed style, to figure out if it is still showing or not.

  it("hides the title image when it's clicked", function () {
    var title = document.querySelector(".title");
    title.click();
    expect(window.getComputedStyle(title).display).toBe("none");
  });

That gives us a test that accurately shows us what’s failing. https://jsfiddle.net/pmw57/ffvkbLjw/326/
But, when the test is run it changes how the button looks. That’s not good, so we can use an afterEach() function to return things back to how they should be, after each test.

  afterEach(function () {
    var title = document.querySelector(".title");
    title.setAttribute("class", "title");
  });

That’s partly effective, but we really need to return the wrap element back to how it started too.

  afterEach(function () {
    var wrap = document.querySelector(".wrap");
    var title = document.querySelector(".title");
    wrap.setAttribute("class", "wrap inactive");
    title.setAttribute("class", "title");
  });

And the button returns back to looking how it should be, after the test.
We do have some duplication now of the title variable, which is going to get worse as testing occurs.

The best-practice technique of dealing with that is to first declare the variable, then inside of a beforeEach() function assign the variable.

  var wrap;
  var title;
  beforeEach(function () {
   wrap = document.querySelector(".wrap");
   title = wrap.querySelector(".title");
  });

We can now remove those var statements from throughout the rest of the code too.

  afterEach(function () {
    // var wrap = document.querySelector(".wrap");
    // var title = document.querySelector(".title");
    ...
  });
  it("hides the title image when it's clicked", function () {
    // var title = document.querySelector(".title");
    ...
  });

There’s one final piece of cleaning up to do. The test triggered the initialOverlay click handler, which then removed itself from the title image. We can deal with that by having a reset() function in the code, and running that after each test. By running that reset() code, we can then move the setAttribute code out of the test, and in to that reset() function instead where it more appropriately belongs.

To make it easier for us to access the button code, I’ve added the button code to a generic app.

var app = {};
app.buttone = (function iife() {

We can then return from the function, anything that we want to make accessible.

  function reset(container) {
    var title = container.querySelector(".title");
    container.setAttribute("class", "wrap inactive");
    title.setAttribute("class", "title");
    container.removeEventListener("click", playButtonClickHandler);
    container.addEventListener("click", initialOverlayClickHandler);
  }
  
  return {
    reset
  };
}());

Normally all of these changes don’t need to be all made at the same time, but because we are dealing with untested code, it hasn’t been written so that it’s easy to test. Starting with tests when you write code results in code that is much easier to test and maintain too.

For now though, the (currently failing) test is in place, and we are in a good situation to work on making the test pass.
https://jsfiddle.net/pmw57/ffvkbLjw/347/

It’s fixed already.

I’m done working on that code.

The title image is expected to turn from a display of block to none when you click on it, but currently that’s not happening. Why is that not happening?

Investigating the element using the browser development tools, I see that the hide class is being ignored, because there’s a style declaration with greater specificity that takes priority over the hide one.

.wrap .title {
    display: block;
}

That style declaration is not needed and can be removed.

/* .wrap .title {
    display: block;
} */

Does removing that style declaration fix the problem? The test passes, so we are all good to move on to the next thing.

Looking at the browser console, I now see:

Uncaught TypeError: Cannot read property 'querySelector' of undefined
    at getPlay ((index):220)
    at isPlaying ((index):242)
    at togglePlayButton ((index):294)
    at playButtonClickHandler ((index):308)
    at HTMLDivElement.initialOverlayClickHandler ((index):316)
    at jasmine.Spec.<anonymous> ((index):347)
    at jasmine.Block.execute (VM2300 jasmine.js:1064)
    at jasmine.Queue.next_ (VM2300 jasmine.js:2096)
    at VM2300 jasmine.js:2086

Clicking on the getPlay() index I see that the undefined button variable is passed in to that function, so I click on the isPlaying() index from the console, to see that the undefined button variable is passed in to that function, and so on until we get to the playButtonClickHandler() function.

  function playButtonClickHandler(evt) {
    var button = upTo(evt.target, ".playButtone");
    togglePlayButton(button);
  }

What is supposed to happen that is currently not happening? The play button is supposed to start playing which shows the pause icon. That pause icon starts off as hidden, so we can check if that’s visible after clicking the initial image too.

Sometimes it can be tempting to place different tests in the same test, but when something goes wrong it’s much more difficult to figure out which of those tests resulted in the failure. So, a new test is added for this next test.

  it("shows the pause icon when the title is first clicked", function () {
    var pauseIcon = wrap.querySelector(".pausee");
    title.click();
    expect(window.getComputedStyle(pauseIcon).display).not.toBe("none");
  });

Here’s the updated code with that failing test: https://jsfiddle.net/pmw57/ffvkbLjw/373/

We can now work on making that failing test pass.