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

Glasgow - Class 06 - Malkit Benning - TV Show DOM Project #1

Open
wants to merge 16 commits into
base: for-review
Choose a base branch
from

Conversation

malkitbenning
Copy link
Owner

for review - I complete up to and including level 500

@netlify
Copy link

netlify bot commented May 26, 2023

Deploy Preview for cyf-malkitbenning-tv ready!

Name Link
🔨 Latest commit 08dced2
🔍 Latest deploy log https://app.netlify.com/sites/cyf-malkitbenning-tv/deploys/6470c0df7ddc680008bbbd53
😎 Deploy Preview https://deploy-preview-1--cyf-malkitbenning-tv.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@ali-nasir-ali ali-nasir-ali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very well-written modular code which is easy to read and follow.

font-family: 'Roboto', sans-serif;
}

.header-element{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CSS code is very readable and with small clearly named elements

<link href="https://fonts.googleapis.com/css2?family=Roboto:wght@300;400&family=Sorts+Mill+Goudy:ital@0;1&display=swap" rel="stylesheet">
</head>

<body>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is quite an amazing way to separate and make the code more modular. I was struggling with this part.

@@ -0,0 +1,246 @@
//You can edit ALL of the code here

function fetchAndBuildEpisodes(tvShowNumber) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thos fetching function is written very well, it's easy to read and the functionality is very well written.

episodesContainer.classList.add("episodes-container");
showElem.appendChild(episodesContainer);

for (const [i, episode] of episodeList.entries()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the parts of the code are written in smaller parts, I should start doing this in my code as well. Its easier to read.

return lowerCaseGenres.includes(matchString);
}

function searchFilter(allShows) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was struggling with this search function you have writen it very smartly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants