2
\$\begingroup\$

This is my first month learning HTML and CSS, I have built a working download page for one of my Java projects and I wanted to see if there is anything to improve on my website.

The website works as I expect, but I'm unsure if I'm doing anything wrong.

Here is my code snippet, any feedback is appreciated!

* {
  margin: 0;
  padding: 0;
}

body {
  font-family: "Montserrat", sans-serif;
}

.container {
  animation: transitionIn 1s;
  height: 100vh;
  width: 100%;
  background-image: url(../resources/images/background.jpeg); 
  background-position: center;
  background-size: cover;  /* auto, cover*/  
  padding-left: 5%;
  padding-right: 5%;
  box-sizing: border-box;
  position: relative;

  background-repeat: no-repeat;
  background-attachment: fixed;
}

.logo {
  width: 120px;
  cursor: pointer;
}


/* loading animation */
@keyframes transitionIn {
  from {
    opacity: 0;
    transform: rotateX(-10deg);
  }

  to {
    opacity: 1;
    transform: rotateX(0);
  }
}

.center {
  margin: auto;
  width: 50%;
  text-align: center;
  padding: 50px;
  line-height: 60px;
}

.center p {
  line-height: 50px;
  font-size: 20px;
}

.shadow {
  -moz-box-shadow: 3px 3px 5px 6px #ccc;
  -webkit-box-shadow: 3px 3px 5px 6px #ccc;
  box-shadow: 3px 3px 5px 3px rgba(0, 0, 0, 0.4);
}

.img-row img {
  max-width: 60vh;
  height: auto;
}

.img-row {
  display: flex;
  padding: 5%;
  justify-content: center;
}

.img-column {
  flex: 33.33%;
  padding: 5px;
}

/* top banner */
.banner {
  background-color: #ffc87c;
}

.banner__content {
  padding: 16px;
  max-width: 1000px;
  margin: 0 auto;
  display: flex;
  align-items: center;
  justify-content: center;
}

.banner__text {
  line-height: 1.4;
  font-size: 2vh;
}

.banner__close {
  background: none;
  border: none;
  cursor: pointer;
  width: 30px;
  height: 30px;
}


@media only screen and (max-device-width: 480px) {
  .container, .banner {
    float: none;
    width: auto;
  }

  .center {
    font-size: 10px;
    width: auto;
    padding: 0%;
  }

  #head_text {
    font-size: 2em;
  }

  #head_warning {
    font-size: 20px;
    line-height: 2em;
  }

  .center h2 {
    font-size: 1em;
    line-height: normal;
  }

  .center p {
    line-height: 20px; font-size: 1em;
  }

  .img-column img {
    max-width: 40vh;
    
  }

  .img-row {
    flex-direction: column;
  }
}
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Project Name</title>
    <link rel="preconnect" href="https://fonts.gstatic.com" />
    <link
      href="https://fonts.googleapis.com/css2?family=Montserrat:wght@300&display=swap"
      rel="stylesheet"
    />
    <link rel="stylesheet" href="./styles/style.css" />
    <link
      rel="shortcut icon"
      type="image/png"
      href="resources/images/project_logo.png"
    />
  </head>
  <body>
    <div class="container">
      <div class="banner">
        <div class="shadow">
          <div class="banner__content">
            <div class="banner__text">
              <strong>Newest Release:</strong> v1.0.1 on 4/21/21
            </div>
            <button class="banner__close" type="button">&times;</button>
          </div>
        </div>
      </div>

      <div class="center">
        <h1 id="head_text">Try Out Project</h1>
        <h2>
          Install Java
          <a href="https://java.com/en/download/" target="_blank">Here </a>

          <h1 id="head_warning">
            If you have Java installed, download the Project zip file
            <a
              href="resources/files/Project.zip"
              download
              style="line-height: 100px"
              >Download Project</a
            >
          </h1>
        </h2>

        <p>
          Please extract the contents of the zip file, and run the .JAR file.
          <br />
          For more information on running a JAR file, please refer
          <a
            href="https://www.computerhope.com/issues/ch001343.htm"
            target="_blank"
            >here.</a
          >
        </p>

        <!-- images -->
        <div class="img-row">
          <div class="img-column">
            <img
              class="shadow"
              src="resources/images/project_home.jpg"
              alt="project demo"
            />
          </div>
          <div class="img-column">
            <img
              class="shadow"
              src="resources/images/project_add_class.jpg"
              alt="project demo"
            />
          </div>
          <div class="img-column">
            <img
              class="shadow"
              src="resources/images/project_error.jpg"
              alt="project demo"
            />
          </div>
        </div>

      </div>
    </div>
    <script>
      // removes banner on close
      document
        .querySelector(".banner__close")
        .addEventListener("click", function () {
          this.closest(".banner").remove();
        });
    </script>
  </body>
</html>

\$\endgroup\$
0
\$\begingroup\$

Not too much jumps out; some comments:

  • You're missing your DOCTYPE
  • On the balance, if I were to be picky &times; is more style than content where you've used it. You can leave your button empty and do a CSS content that achieves the same thing.
  • It seems very strange to have an h1 embedded in an h2. You probably shouldn't do that.
  • To be consistent with your own style, and to maintain legibility, do not break your </a> onto multiple lines
  • Pixels as measurement units in CSS have a contorted, confusing and nonintuitive meaning. Read e.g. this article. The days of browsers actually rendering one pixel as one pixel have been over for many years; so just don't write your measurements this way.
\$\endgroup\$
2
  • \$\begingroup\$ Thank you! The multi-line </a>'s are from the prettier extension. I just assumed it was a best practice. I'll definitely change it. \$\endgroup\$ – Alex Wang Apr 24 at 17:48
  • \$\begingroup\$ It's OK to have an anchor on multiple lines; just don't break the closing tag onto multiple lines. \$\endgroup\$ – Reinderien Apr 24 at 17:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.