# Tag Info

12

Some comments on your style of selecting: div.pagination is overqualified. A container is almost always going to be a div, so you just can write .pagination In many cases, there is no use for an extra container. You might as well apply the pagination class to the list itself: <ul class="pagination"> You also have a lot of redundancy in your selectors ...

6

This uses mappings, which are part of Sass 3.3. You can do the same thing without mappings, it's just not as pretty. In addition to allowing multiple selectors, it allows for more options than just light/dark colors (eg. background images, etc): $themes: ( blog: ( default: red , light: lighten(red, 20%) ) , portfolio: ... 5 Personally, I think it's better to make the breaks according to the content of the site, rather than to a fixed size. This is even more true nowadays with docens of different screen sizes for each type of device. You should make sure the site will always look good by making the breaks work depending on the design, and no the other way around. 5 Some days passed and I feel like I should share the changes I have made so far. Naming I started using page names instead of color names on classes and variables. This leaves me with the benefit of being able to change the color scheme of an entire page only by changing the value of the color variable. .page--home -> .page--blog .nav__item--green ... 5 I found this post and took your code and made some changes. I wanted to have a very small piece of javascript code that supported style-able tooltips. First, the way you were editing the CSS was to add an additional style every time the mouse moves. This will create hundreds of style additions in just a few seconds of mouse movement, so very inefficient ... 4 this popped out at me, I haven't looked at the entire code completely yet, so take this with a grain of salt please not sure why you do this <a href="#member-name-2" class="block-list__link"> <div class="block-list__thumbnail"> <img src="member-name-2-thumbnail.jpg" alt="Thumbnail"> </div> <span class="block-... 4 Overall, your naming conventions are pretty good. I don't feel like I need to go look at mixins themselves to figure out what their purpose is. The extensive use of extends does concern me, since it can lead to larger CSS rather than smaller like you might expect (see: Mixin, @extend or (silent) class?). Your %textOnDarkBg and %textOnRedBg extend classes ... 4 For me, a lot of this looks like you're trying to... Sassify it too much. Let's start by looking at your padding mixin: @mixin padding($value: 0, $top: 0,$right: 0, $bottom: 0,$left: 0, $omit: null) { @if$top == 0 and $right == 0 and$bottom == 0 and $left == 0 { padding:$value; } @else if $omit != null { padding:$top $omit } @else if$top ...

4

I see some Redux conventions misconceptions: The reducer purpose is to change the state, I see that you also copy state to localStorage. You should use actionCreators to save data in localStorage. In actionCreator, you save data and then - dispatch action to reducer. Any of the React components with Redux don't need to have state. You should pass all ...

4

Should I try to concentrate more on JavaScript only? I mean: Should I try to reduce the mandatory HTML-, CSS-code to a minimun? I think, the code i.e. HTML and CSS are minimal. Storing the tooltip on the HTML elements is correct way rather than storing it in JavaScript. So, NO. You should not only concentrate on JavaScript. Or is Okay the way I have done ...

3

import HelpMe from '../helpers/helpers'; const __ = new HelpMe(); This doesn't tell me what HelpMe is unless I look for it. Same can be said for __. We don't know what it is. It's not like jQuery or Underscore where they have been known for quite a while to use $and _, respectively. Best if you just named it Helper. __.forEach Not sure why you need this. ... 3 Some things that I would do differently in this for loop for(var i=0; i < ripples.length;i++ ){ ripples[i].addEventListener('click', function(e){ var rippleThis = this; var parentOffset = offset(rippleThis.parentNode); var x = e.pageX - parentOffset.left; var y = e.pageY + parentOffset.top; var j; var ... 3 Feedback The code looks pretty good. The Javascript code waits until the DOM is ready, then caches references to DOM elements in variables before using those variables in the functions later on. I initially thought some of the CSS could be consolidated (e.g. many common styles under .menu-link and .menu a but then I realized that might be tricky with the ... 3 There is no real reason to use JavaScript here. Using a CSS media query you can do this in a few lines: #menu { display: block; } [href="#menu"] { display: none; } @media (max-width: 800px) { // rules only apply if screen width is up to 800px wide #menu { display: none; } [href="#menu"] { display: block; } } 3 It just looks really good. I think it stayed this long unanswered because there's not much feedback to give. I reviewed and liked The commenting The naming The sizes of your functions Reading flow I am not convinced that choosing a random int actually helps the load balancing much, but don't have a better alternative. I would also probably put the random ... 3 Move all your duplicate code into a class, say gradient. Pick better names. A class name like light-green can mean anything, can be put on anything and really is not that great. I like Microsoft's (Fabric UI) method to fix this. You define your base class, say gradient, and then add your customizations via a subclass, say gradient--light-green. The double ... 3 Manipulating DOM with VueJS The biggest comment I have about this code is that it modifies DOM elements using vanillaJS techniques. With VueJS, there are ways of achieving this without needing to reference those elements via properties like id or ref. One way is to use computed properties and bound styles. See rewritten code below for examples. Other ... 2 You can DRY your use of @font-face using this mixin see on codepen but you'll get better results using compassor bourbon, etc... // Use variables instead names (name that vars as you wish, because I don't know which of them are the most used)$primary-font: baskerville_old_faceregular; $secondary-font: copperplate_gothic_boldRg;$tertiary-font: ...

2

Sass If this is meant to be a reusable Sass library, you'll want to make use of default variables so that they're easier to override: $container-width: 940px !default;$search-container-padding: 54px !default; You'll want to do something similar with your mixins so that the values can be changed for one-off instances: @mixin input-width($width,$form-...

2

Why are you doing this in easeOutCubic? return c * ((t = t / d - 1) * t * t + 1) + b; that's way harder to read than something like t = t / d - 1; return c * (t * t * t + 1) + b; // or return c * (Math.pow(t, 3) + 1) + b; Sorry I don't know a ton about jQuery or SVG or I'd try to help more, but that was really bothering me.

2

You can minimize your SASS code using @each and nesting for :hover .parent-div a { border-bottom: 10px solid; @each $color in blue, red, green, yellow, pink, purple { // Iterate through colors .#{$color} & { border-color: $color; &:hover { // Nested inside border-color: darken($color, 10%); } } } } which ...

2

Surely $(elem).width() / 4 is already defined as a <circle> attribute so why not read it from the DOM in the same way as percentage? Alternatively, simplify the HTML down to the <figure></figure> wrapper and generate its <svg>...</svg> content dynamically. The legend doesn't appear in either Opera or Chrome. 2 Whitespace in expressions There is a distinct lack of whitespace in all of your expressions in Sass (eg.$colwidth: $i/12*$totwidth). This has been known to cause confusion with the parser in certain versions of Sass (notably with subtraction). I would recommend always including whitespace around every arithmetic operator, rather than just the ones that ...

2

I can't say much for flexbox, I haven't played with it. The React part is to sparse as well. However, general CSS I can do. .c-marketview { display: flex; flex-flow: column nowrap; max-width: 1300px; min-width: 850px; margin: auto; * { box-sizing: border-box; } } I suggest avoiding * and apply only to the elements you ...

2

You could remove all the duplicate lines in the @media queries and provide only the changes in these tags. This is beacause in CSS, the CSS classes that are interpreted by the browser down-most on your page, are the ones prevailing in your final design. All previous defined CSS rules apply unless overwritten. In your case this means that only the font-size ...

2

Your way works, but it is not very scalable, since you have to compile all the sass files every time you make a change, this is time consuming when the project gets bigger. here you watch all the files via grunt/gulp, but only compile the _watchables, the _procompiled file is compiled upon start of server only, not during file watch. when building for ...

2

Just few things to highlight here Misusing Twitter bootstrap: The grid structure in bootstrap allows up to 12 columns across the page. <div class="col-xs-7 col-sm-4 col-md-3"> Here you have define 14 columns which is wrong. I will assume you don't know the four classes of Grid Layout in Bootstrap : xs (for phones), sm (for tablets), md (for ...

2

Here are some improvements: Use IIFE. This will make the variables defined inside it private thus avoid cluttering globals Passing window and document objects will make it available in current scope and the scope chaining to find them will be reduced, improving performance slightly. Allows to create private and public variables and methods. Customizations ...

Only top voted, non community-wiki answers of a minimum length are eligible