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

Saphire C-19 Jen Tam, Edith Baltazar, Yulia S #37

Open
wants to merge 67 commits into
base: main
Choose a base branch
from

Conversation

ystartc
Copy link

@ystartc ystartc commented May 19, 2023

Group co-working project

JenniferWTam and others added 29 commits May 17, 2023 15:48
added banner to home and changed home button to img
Added image backround, allign text, changed font family
Copy link

@mmcknett-ada mmcknett-ada left a comment

Choose a reason for hiding this comment

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

Nice job on your site! It's very creative and playful!

I think it's a great idea to run the W3C validator and check that the HTML structure is valid. Most of my notes are about indentation, which I think might help you notice various issues with the HTML validity. This will apply in JSX inside of React, too, so hopefully these notes help with debugging!

It's also a good idea to think about accessibility beyond semantic HTML. Two things I highlighted there:

  1. Text Contrast (making sure the text is visible enough against its background
  2. Mobility issues (can a slower mouse user interact with moving elements?)

Comment on lines +18 to +25
<section class="topnav">
<!-- Centered link -->

<nav class="topnav-centered">
<button href="index.html">
<img id="home" class="active" src="/images/EJY.jpg" />
</button>
</nav>

Choose a reason for hiding this comment

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

[nit] It's a good idea to consistently indent siblings! That way you can tell whether things are nested without searching for closing tags.

    <section class="topnav">
      <!-- Centered link -->

      <nav class="topnav-centered">
        <button href="index.html">
          <img id="home" class="active" src="/images/EJY.jpg" />
        </button>
      </nav>

Comment on lines +46 to +50
<a href="us.html">About Us</a>
<form class="d-flex">
<input class="form-control me-2" type="search" placeholder="Search" aria-label="Search">
<button class="btn btn-outline-success" type="submit">Search</button>
</form>

Choose a reason for hiding this comment

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

This example illustrates the indenting. As-is, I'm wondering where the </a> is at the end of the form. But the form isn't enclosed in the anchor! Try indenting like this:

          <a href="us.html">About Us</a>
          <form class="d-flex">
            <input class="form-control me-2" type="search" placeholder="Search" aria-label="Search">
            <button class="btn btn-outline-success" type="submit">Search</button>
          </form>

It typically consists of a tea base, milk or a non-dairy alternative, and tapioca pearls or other chewy toppings.
</h3>
</article>
<section id="backgroung-image">

Choose a reason for hiding this comment

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

Great use of semantic tags throughout!

Another accessibility consideration is text contrast. Notice how little the text stands out against the background image in some places:

Article screenshot showing low text contrast between background image and text

Think about some strategies you might use to increase the contrast between the text and the background image!

<strong>Brown Sugar:</strong> Brown sugar boba is a trendy variation where the pearls are cooked in a rich brown sugar syrup. This gives them a caramel-like flavor and a chewy texture.
</li>
</ul>
<h3 id="boba-types">

Choose a reason for hiding this comment

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

This h3 might be better off as a paragraph with different styling, since it doesn't seem to be the heading of a separate piece of content.

</section>

<footer class="footer">
<section>

Choose a reason for hiding this comment

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

[nit] Since this is all in the footer, and it's the only section in the footer, you probably don't need a section here.


</style>
</head>
<body>

Choose a reason for hiding this comment

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

[nit] Only use one <body> per HTML file.

Comment on lines +127 to +130
</li>
</td></tr>
<tr><td>
<li> <img class="matcha-gif" src="https://media3.giphy.com/media/1yniKGdw6TiVk5s7pl/200w.gif?cid=82a1493bqhzpj78bl051sg2obv56q048fs5aeqz8278ouyew&ep=v1_gifs_related&rid=200w.gif&ct=s" alt="tb"></a>

Choose a reason for hiding this comment

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

Indenting will help detangle some of this:

                </li>
              </td>
            </tr>
            <tr>
              <td>
                <li>
                  <img class="matcha-gif" src="https://media3.giphy.com/media/1yniKGdw6TiVk5s7pl/200w.gif?cid=82a1493bqhzpj78bl051sg2obv56q048fs5aeqz8278ouyew&ep=v1_gifs_related&rid=200w.gif&ct=s" alt="tb">
                </a>

This may make you see a couple of things:

  • There's an li but no surrounding ul for it
  • There's a stray </a> tag

Comment on lines +31 to +36
<nav class="dropdown">
<button class="dropbtn">Gallery</button>
<nav class="dropdown-content">
<a href="gallery.html">Boba Tea Gallery</a>
</nav>
</nav>

Choose a reason for hiding this comment

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

There's a lot of navs! I think you'd use nav instead of section up at the "Top navigation" comment.

<img class=covers alt="" src="https://ameessavorydish.com/wp-content/uploads/2023/01/Matcha-milk-tea-post-7.jpg"> Matcha Milk Tea</a>
</li>
<li>
<a href = "https://www.amazon.com/Clean-Ruby-Crafting-Better-Rubyists-ebook/dp/B0825PGLR2/ref=sr_1_1?crid=3K622H88RR4KI&keywords=clean+ruby&qid=1684189598&s=digital-text&sprefix=clean+ruby%2Cdigital-text%2C293&sr=1-1">

Choose a reason for hiding this comment

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

Your bubble tea images link to coding books on Amazon. 😄

Copy link
Author

Choose a reason for hiding this comment

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

just checking if u pay attention 😅

Choose a reason for hiding this comment

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

it be like that sometimes - never let them know your next move

Comment on lines +18 to +29
#about-me {
margin-top: -75px;
text-align: center;
background: -webkit-linear-gradient( #ffffcc, #FFB6C1);
-webkit-background-clip: text;
-webkit-text-fill-color: transparent;
text-shadow: -1px 1px 2px #FFB6C1,
1px 1px 2px #ffffcc,
1px -1px 0 #ffffcc,
-1px -1px 0 #ffffcc;
/* padding: 50px; */
}

Choose a reason for hiding this comment

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

[nit] Definitely indent these

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.

3 participants