vrajkumar128 / Colmar-Academy

0 stars 0 forks source link

CSS/JS #3

Open Axkaban opened 6 years ago

Axkaban commented 6 years ago

https://github.com/vrrajkum/Colmar-Academy/blob/master/resources/css/style.css#L3-L11

Great use of global/universal styles, one selector you could use instead of html is the ‘all’ selector: * or simply body, since all your elements are inside it.

for grouped elements follow Airbnb’s style guidelines since they are standard across the industry. i.e..

    h2, 
    h3 {
      font-family: "Arial", sans-serif;
      font-weight: bold;
      color: #EEB111;
    }

Great use of flex box and responsive sizing (em/rem) have you look into grid? You might like it a lot!

About modernizr, is that a copy and paste or you wrote it? somehow it shows on your repo like a run on sentence, since it is JavaScript(and I believe you haven’t seen much of it) you can rewrite it like this:

!function(e,n,t){
  function s(e,n){
    return typeof e===n
  }
  function o(){
    var e,
    n,
    t,
    o,
    a,
    i,
    l;
    for(var c in f)if(f.hasOwnProperty(c)){
      if(e=[],n=f[c],n.name&&(e.push(n.name.toLowerCase()), n.options&&n.options.aliases&&n.options.aliases.length))
        for(t=0;t<n.options.aliases.length;t++)e.push(n.options.aliases[t].toLowerCase());
          for(o=s(n.fn,"function")?n.fn():n.fn,a=0;a<e.length;a++)i=e[a],l=i.split("."),1===l.length?Modernizr[l[0]]=o:(!Modernizr[l[0]]||Modernizr[l[0]]instanceof Boolean||(Modernizr[l[0]]=new Boolean(Modernizr[l[0]])),Modernizr[l[0]][l[1]]=o),r.push((o?"":"no-")+l.join("-"))
        }
  }
            function a(e){
              var n=c.className,t=Modernizr._config.classPrefix||"";
              if(u&&(n=n.baseVal),Modernizr._config.enableJSClass){
                var s=new RegExp("(^|\\s)"+t+"no-js(\\s|$)");
                n=n.replace(s,"$1"+t+"js$2")
              }
              Modernizr._config.enableClasses&&(n+=" "+t+e.join(" "+t),u?c.className.baseVal=n:c.className=n)
            }
            function i(){
              return"function"!=typeof n.createElement?n.createElement(arguments[0]):u?n.createElementNS.call(n,"http://www.w3.org/2000/svg",arguments[0]):n.createElement.apply(n,arguments)
            }
            var r=[],
            f=[],
            l={
              _version:"3.5.0",
              _config:{classPrefix:"",
                        enableClasses:!0,
                        enableJSClass:!0,
                        usePrefixes:!0},
              _q:[],
              on:function(e,n){
                var t=this;
                setTimeout(function(){n(t[e])},0)
              },
              addTest:function(e,n,t){
                  f.push({name:e,fn:n,options:t})
                },
              addAsyncTest:function(e){
                  f.push({name:null,fn:e})}
                },
              Modernizr=function(){};
              Modernizr.prototype=l,
              Modernizr=new Modernizr;
              var c=n.documentElement,
              u="svg"===c.nodeName.toLowerCase(),
              g=l._config.usePrefixes?" -webkit- -moz- -o- -ms- ".split(" "):["",""];
              l._prefixes=g,Modernizr.addTest("cssgradients",function(){
                for(var e,n="background-image:",t="gradient(linear,left top,right bottom,from(#9f9),to(white));",s="",o=0,a=g.length-1;a>o;o++)e=0===o?"to ":"",s+=n+g[o]+"linear-gradient("+e+"left top, #9f9, white);";
                  Modernizr._config.usePrefixes&&(s+=n+"-webkit-"+t);
                var r=i("a"),
                f=r.style;
                return f.cssText=s,(""+f.backgroundImage).indexOf("gradient")>-1
              }),
              o(),
              a(r),
              delete l.addTest,
              delete l.addAsyncTest;
              for(var d=0;d<Modernizr._q.length;d++)Modernizr._q[d]();
                e.Modernizr=Modernizr
            }
            (window,document);
vrajkumar128 commented 6 years ago

@Axkaban I've noticed that * overrides the default font sizes for header elements (h1, h2 etc.), so I avoid it. My understanding is that the global font size should be specified on html rather than body, because html is the root element from which the rem unit is computed.

I did write my grouped selectors the Airbnb way, but my CSScomb plugin must've changed them and I didn't notice.

I actually did use display: grid in my #learn #subjects selector.

The polyfill that I used for gradients was automatically generated by https://modernizr.com/, I didn't write it myself. I don't think there's really any need for it to be human-readable.

Axkaban commented 6 years ago

It does, you're right, but in your html you only pass the parameter of font-family, in general it is a sugestion, and you made a very good observation to it. Now, it sucks that the plugin re-arranged your grouping. Glad you noticed. I am sorry, I completely passed by the grid on #learn #subjects, I went back and I got to give it to you! You're certainly rocking all the tools at your disposition. On the js, it is true that it is auto generated, you may just want to make that comment for those of us not acquainted with modernizr.