qassisju / MI-449-SS17-741-js-collect-and-display-information-kPt9Fl

0 stars 0 forks source link

Project Feedback #1

Open qassisju opened 7 years ago

qassisju commented 7 years ago

@KatieMFritz Can you take a look at this? It's hosted here and meets the following criteria:

KatieMFritz commented 7 years ago

Hey Justin! Nice work figuring this out. 🔥 💥

Your addCode function does great at publishing my input into the profiles, but there are some formatting issues with the profile output.

And a tip, that will make that first change easier. 😄

Since you're assigning the exact same HTML string twice, what would you think about assigning it to a variable first, then reusing the variable? So for example, if I'm doing this:

element1.textContent = 'hello'
element2.innerHTML = 'hello'

Then whenever I want to update 'hello', I have to do it in two places. We're not only too lazy for that 😄, but it can also cause bugs (especially if 'hello' is a longer, more complex string - say, an entire dating profile). If we update string and forget to update the other the same way, then we have an inconsistency. A solution for this would be to move 'hello' to a variable, like this:

var greeting = 'hello'

element1.textContent = greeting
element2.innerHTML = greeting

Now whenever we want to update the 'hello', we can do it in just one place. Future development is now much easier and it's more difficult to create bugs. Does that make sense?


After you’ve made your changes and pushed them to GitHub and your hosted site, give it a once-over to make sure it looks right, then comment back here and I’ll take another look.

Thanks! :rocket:

qassisju commented 7 years ago

The spaces have been added and the variables have been changed

KatieMFritz commented 7 years ago

Nice job iterating! That new variable seems to be working great and the spaces all look good now. ✨ ✨

I'm still getting this problem:

The raw HTML displays as one long line, forcing me to scroll horizontally. 😮

Here's a hint: I notice it also shows up as preformatted text, which is what I expect, but it doesn't appear in a code block (gray background) once I start typing. Check out your index.html and see if you can figure out what's going on. Slack me if you get stumped. 🤔

qassisju commented 7 years ago

Got it, check now

KatieMFritz commented 7 years ago

YES YOU ARE A WIZARD. 😁 Great detective work!

:shipit: