yufenhsi / MI-449-SS18-740-js-collect-and-display-information-kPt9Fl

0 stars 0 forks source link

Project Feedback #1

Open yufenhsi opened 6 years ago

yufenhsi commented 6 years ago

Build a dating profile generator

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

KatieMFritz commented 6 years ago

Hi Yufen, excellent start! 🎉 Your JavaScript is very efficient, and your app works almost exactly right. 💪

Here are some ways to improve:

1. Use the HTML we provided for the profile

Your profile is close to ours, but we'd like you to use the exact HTML we provided in profile.html. This includes an h1 element and two p elements. (You will still need to replace our placeholders like EMAIL_ADDRESS with your own JavaScript variables, but that's the only changes you should make.)

You may have noticed that putting that code inside another p element creates invalid HTML. However, you can safely put it inside a div! Try replacing the innerHTML of the div in the Preview section, instead of adding a new element.

Similarly, you've put the generated HTML inside an em element, which makes it all italicized. Try replacing the textContent of the code element instead! It should work much better. 🚀

2. Fix contact links

Your email and phone links do not go to the email address or phone number that the user provides. Instead, they send an email to EMAIL_ADDRESS or a phone call to PHONE_NUMBER. See if you can figure out why, and how to fix it! If you get stuck, I can help. 😄 ☎️ ✉️

Note: the punctuation for this can be confusing! Pay close attention to the punctuation and to any linter errors you get. Also, you can usually see where a link will go if you hover your mouse over it and look in the lower left-hand corner of your browser window.

3. Use more descriptive variable names

Many of your variables, like firstNameInput or email, have great names; I know exactly what they are. However, you have a few variable names that don't really make sense to me:

Don't forget to update the HTML id attributes for these elements too!


I'm on Teams all afternoon if you have questions! 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:

KatieMFritz commented 6 years ago

I made a few updates to my first comment, because I submitted it too soon. 🙃

yufenhsi commented 6 years ago

Hi Katie,

Thank you! I used the HTML provided for the profile and both email address and phone number can work now after I've replaced the code in the HTML instead old one. Also, I've switched those variables which do not make sense to a reasonable name. Sorry for using the variable name of this lesson, I will avoid making this sort of mistakes next time.

KatieMFritz commented 6 years ago

Hi Yufen, awesome commit message! 🔥

Your project is getting closer! 😄 Your new variable names make much more sense to me. Nice job. 👍

1a. Put your PROFILE_PREVIEW and HTML-CODE on valid elements

You created new p elements to put your profile preview and generated HTML in:

          <h2>Preview</h2>
          <div>
            <p id="PROFILE_PREVIEW"></p>
          </div>

          <h2>Generated HTML</h2>
          <div>
          <pre><code>
            <p id="HTML-CODE">THIS IS WHERE THE RAW HTML SHOULD APPEAR</p>
          </code></pre>
          </div>

It looks like you also added a div for the Generated HTML part.

This code structure leads to invalid HTML. You're getting a validator error, and as I explained in my first comment, it's invalid to nest a p inside another p.

What you did is create new elements and replace their innerHTML and textContent. Instead, please assign your PROFILE_PREVIEW and HTML-CODE ids to the elements that were there from the beginning, like this:

        <div class="six columns">
          <h2>Preview</h2>
          <div id="PROFILE_PREVIEW">
            THIS IS WHERE THE HTML PREVIEW SHOULD APPEAR
          </div>
        </div>
        <div class="six columns">
          <h2>Generated HTML</h2>
          <pre><code id="HTML-CODE">
            THIS IS WHERE THE RAW HTML SHOULD APPEAR
          </code></pre>
        </div>

If you're confused about why, message me on Teams and we can talk about it more! 😸

1b. Clean up profile a little more

This is what your profile looks like now:

You are getting closer with the profile code, but there are still a couple things that are off.

  1. "DESCRIBE_YOURSELF_INFO" shows up above the description that I typed ("I like cats.")
  2. There are some missing spaces around the variables. Remember that you can put a space in a string by itself, like this: ' ', or add it on to an existing string, like 'email me at '. 👾

2. Fix contact links

See my previous comment for details. I still see the same problem. ☎️ ✉️

3. Use camelCase for JavaScript variable names

Remember back in the first JavaScript lesson, you learned about camelCase? That's when you begin with lowercase, then each word after the first word starts with a capital letter.

Your first several variables, like rawCode, are written perfectly in camelCase. Inside your updateProfile function, though, your variables are written LIKE_THIS. Could you please update those so they all use camelCase? Thanks! 🐫

Optional bonus challenge: For HTML id attributes, usually we write them in kebab-case, where all words are in lower case with hyphens (-) separating them. If you want, you could update your HTML ids to look like phone-number instead of PHONE_NUMBER. It's up to you though! 😄


Sorry this is so much text! Go step by step, and message me on Teams if you want me to explain it differently. 🚀

yufenhsi commented 6 years ago

Hello Katie,

Thank for your patience to give those suggestions. Text is not so much and really helpful.
I've used ' ' or make a space to separate texts when preview. I also fixed contact links and I can see the email and tel show in the left-hand corner of my browser window.

Also, I replaced JavaScript variable names in camelCase. And I changed all of the HTML ids like from LAST_NAME to lastname. Before that, I had tried to put (-) to look like last-name, but my HTML preview and rawcode cannot work at all. I did not know why and had no idea how to fix it, so I remove all (-) of id names.

KatieMFritz commented 6 years ago

Yay, your links work perfectly now, your spacing is fixed, and your variables are in the standard style! 💥 🎉 😸

Only one small fix left:

  1. "DESCRIBE_YOURSELF_INFO" still shows up in the profile. We should not see that at all, only what the user types.

You are so close! 😁

yufenhsi commented 6 years ago

I've removed the "DESCRIBE_YOURSELF_INFO" and the texts don't show up in the profile.

Thank you! 😁

KatieMFritz commented 6 years ago

:shipit: There it is!!!!

great-job-cats