blogc / blogc.rgm.io

blogc.rgm.io sources.
https://blogc.rgm.io/
BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Utilize the bootstrap classes #1

Open ghost opened 8 years ago

ghost commented 8 years ago

https://github.com/blogc/blogc.rgm.io/blob/master/templates/main.tmpl#L88

From

  <div class="list-group-item" style="text-align: center; font-size: 1.4em">
    blogc-{{ LATEST_RELEASE }}
  </div>

To

<div class="list-group-item text-center">
  <span class="h3">blogc-{{ LATEST_RELEASE }}</span>
</div>

Why is the Jquery and BootstrapJS need ? The latest bootstrap as well bootswatch Flatty release is 3.3.6 , same goes for the font-awesome version.

// inlined directly into the html page
<script>
var daMenus = {

    // Navbar and dropdowns
    toggle: document.getElementsByClassName('navbar-toggle')[0],
    collapse: document.getElementsByClassName('navbar-collapse')[0],

    toggleMenu: function() { // Toggle if navbar menu is open or closed
        daMenus.collapse.classList.toggle('collapse');
    },

    // Close dropdowns when screen becomes big enough to switch to open by hover
    closeMenusOnResize: function() {
        if (document.body.clientWidth >= 768) {
            daMenus.collapse.classList.add('collapse');
        }
    }
};

// dropdown menu listeners
window.addEventListener('resize', daMenus.closeMenusOnResize, false);
daMenus.toggle.addEventListener('click', daMenus.toggleMenu, false);
</script>

custom.css

/*
 * Open dropdowns on hover instead of click.
 */
@media (min-width: 768px) {
  .dropdown:hover {
    background: #e7e7e7;
  }
  .dropdown:hover > .dropdown-menu {
    display: block;
  }
}

/*
 * The following is needed since the dropdowns are <div> elements instead
 * of <a> elements
 */
.nav > li > div {
  position: relative;
  display: block;
  padding: 10px 15px;
  cursor: default;
}
.navbar-nav > li > div {
  padding-top: 15px;
  padding-bottom: 15px;
  line-height: 20px;
}
.navbar-default .navbar-nav > li > div {
  color: rgb(119, 119, 119);
}
.navbar-collapse.collapse {
  display: none;
}
.dropdown-open {
  background: #e7e7e7;
}
rafaelmartins commented 8 years ago

Hi,

thanks for opening this issue.

I'll look at porting my inline style definitions to bootstrap classes.

About bootstrap version, I'm not frontend developer, and don't really want to spend time tracking js library versions, but ok, I can update it.

About "forking" code from bootstrap and putting it inline on my templates, I don't like the idea. I'm already using a CDN, that creates a single js and css bundle for me, and does good caching, then I don't think that this is an issue. and maintaining js/css code isn't something I'll do unless strictly necessary.

Thanks

ghost commented 8 years ago

Ahoi kamarat,

The presented css here was meant to be added in your custom.css, not inlined into the html page itself. It's highly unlikely that the bootstrap devs and version 3 will rename any of the nav and navbar classes which are used to compose the website navigation bar.

Every single visitor will have to download 118KB javascripts bundle of which only the mobile users will be able to utilize the small amount of code required to add "click" action, so the hamburger button to show the rest of the navigation menu as dropdown menu.

Actually after opening your custom.css I can optimize the code even further:

h3,
h4,
h5 {
    margin-bottom: 20px;
}

It doesn't have to become a full-time job to constantly check for the external resource versions, just do it twice in a year, as they may contain critical bug fixes which may affect your website visitors.


One minute later:

The jumbotron class can be combined with text-center, so:

.jumbotron {
    text-align: center;
}

is not necessary.

https://github.com/blogc/blogc.rgm.io/blob/master/content/index.txt#L2

<div class="jumbotron text-center">

the body padding is not necessary

body {
    padding-top: 60px;
}

Just change:

<div class="navbar navbar-default navbar-fixed-top">

to:

<div class="navbar navbar-default navbar-static-top">

Putting it all together:

custom.css

h3,
h4,
h5 {
    margin-top: 20px;
}

.sidebar {
    padding-top: 20px;
}

My blog

ghost commented 8 years ago

Pull request is pending https://github.com/blogc/blogc.rgm.io/pull/2