Skip to content
This repository has been archived by the owner on Dec 27, 2017. It is now read-only.

navbar #7

Closed
wants to merge 2 commits into from
Closed

navbar #7

wants to merge 2 commits into from

Conversation

zavfel
Copy link

@zavfel zavfel commented Mar 13, 2016

Fixes #2

@nicholaslum444
Copy link
Member

Couple issues:

  1. Does not adapt to mobile view (Ctrl-Shift-M on Chrome to test)
  2. The entire left banner should be link to the acad home page (acad.nuscomputing.com)
  3. The entire right banner should be a link to the main website (nuscomputing.com)
  4. Wrap the contents of the body in a container div. i.e.
<body>
  <div class="container">
    // original body contents
  </div>
</body>

Thanks for the work so far!

@@ -3,11 +3,62 @@
<head>
<title>Academics | NUS Computing</title>
<link rel="stylesheet" href="assets/css/bootstrap.css">
<link rel="stylesheet" href="assets/css/stylesheet.css">
<link rel="shortcut icon" type="image/x-icon" href="assets/img/favicon.ico">
<script src="assets/js/bootstrap.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

since we are serving the js from a CDN, you can remove this line

@nicholaslum444 nicholaslum444 self-assigned this Mar 14, 2016
@@ -3,11 +3,62 @@
<head>
<title>Academics | NUS Computing</title>
Copy link
Member

Choose a reason for hiding this comment

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

Add the following lines above the <title> tag

    <meta charset="utf-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />

This will show the mobile view on mobile sized screens (right now it doesn't work that well)

@nicholaslum444
Copy link
Member

So far so good, the desktop view looks good. Just one more correction

However the mobile view is not that great. The top bar elements are all misaligned. This is because they are in their own divs.

For prototype purposes, we'll pull this in. When we decide on the actual framework, then we'll work on the mobile view again.

@kennho Any thoughts? Else I'll merge it after the corrections are made

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

Successfully merging this pull request may close these issues.

2 participants