Hi Joran,
I would expect this kind of challenge to take quite a bit longer than 2 hours of done properly to be honest, and wouldn't recommend pushing yourself for time in this way often.
I've only had a very quick look at the top of the html and a few things jumped out at me straight away.
Your toggle has an empty label rendering it inaccessible, yet you have the words dark mode right there with a class of __label. That should be a label element and if rendering dark mode by default that would be the checked state.
With BEM it looks like you're following the naming convention sometimes and not others. Nearly but not quite. For example
<h1 class="heading-primary">
<span class="heading-primary--main">Social Media Dashboard</span>
<span class="heading-primary--sub">Total Followers: 23,004</span>
</h1>
Firstly, that shouldn't be one heading. Social media dashboard is the h1, but total followers part should be a paragraph.
But with the BEM in that example, you're using modifiers when you mean to be using elements. heading-primary__main would be correct BEM but --main would not in this instance. Modifier classes are only used alongside their original classes to make a change to them. So you might have heading__primary heading__primary--main on one element. It's always block__element--modifier, with the modifier going on the same element that has its matching block__element.
I hope that makes sense, I'm tired tonight and feel like I'm. Not explaining myself well 😂
This is actually a really difficult challenge when it comes to html semantics!
I think your card-total__total-followers needs to be a paragraph as you can't have text in spans and divs alone.
And you need to find a way to communicate the increase or decrease on the following number to assistive technology. I'm genuinely not sure how I'd do that... I would probably put the svgs in the html markup and give them appropriate aria-label text to communicate whether the number has gone up or down.
I can't really comment on the css as I find the formatting really hard to read at the moment. Add some auto formatting / Linting if you can to your code editor and it will give the css some breathing space to make it more easily readable.
Best of luck with it all, you're on the right track with all this I think 👍
@DrKlonk
Posted
@grace-snow @comment 1:
About the toggle: Do you mean inaccessible as in: people with screen readers cannot use it?
The idea is that the label is clickable and handles checking the hidden input checkbox.
I do agree that it makes more sense to have the __label class on the label (#logic). This did mean - in my way of handling things - that I needed to hack the label a bit and that I couldn't really follow the mobile design anymore. The button is now next to the label there, so the clickable area makes more sense.
About BEM: The heading BEM part is something I blatantly copied from the Advanced CSS course (https://www.udemy.com/course/advanced-css-and-sass/), but I do see your point about the modifier, it makes total sense. An item should be that item (block or element) first, before it gets modified.
I wouldn't really think of the sub-heading as a paragraph, though (more thoughts on this below). Would it make sense to use an <h3>? Since it really connects well with the items below it.
@comment 2: First of all: thank you for giving me two elaborate and thorough comments :)
No text in divs or spans alone makes sense too! I was just thinking: well, that is not a paragraph. It is not even a sentence, how can it be a paragraph?
However, I've been checking out this discussion on it (mind you, this is >10 years old) https://www.sitepoint.com/community/t/p-vs-span/5298/12.
And someone mentions in the thread: "The paragraph, as taught in middle school English, is a grammatical construct, usually made up of sentences. The thing is, html is a structural markup language, not grammatical. As a structural element, it is “a brief composition complete in one typographical section.”".
I guess you would adhere to that and put text at the very least in a <p> at all times?
On the increase/decrease: I'm not happy with the way I fixed it, but I figured the number will in practice go through a piece of javascript where the class could be added based on its value. The aria-label is something new to me, will look into it!
On the css: it gets compiled like this by default by the node-sass package. I will look into making it a bit nicer looking.
Thank you so much for your time and helping me out!
