Tip calculator app

Community feedback

• 

  Kristian Kalvå•360

  @kkalvaaPosted almost 2 years ago

  You do need to use IDs for for=…, that is true, but you do not have to attach your css to the ID, even if you do have an ID on an element. You can just add a class as well and hook your css onto that instead of the ID and you'll lower the specificity dramatically. Classes will never be able to override ids without !important, except in an earlier version of a browser (I forget the exact one) where a bug let you chain 256+ classes to override an ID. This has since been patched and will no longer work. If you add classes to your labels, and hook your css to those instead of the IDs, you'll most likely find that you don't need !important anymore.

  You are however right that in the HTML semantics is important and you should be using IDs to connect the labels with the input elements. CSS however isn't semantic and carries no semantic value, you can name your selectors whatever you want and it won't make any difference. Good naming is for you, the developer (and other developers), not browsers and assistive technology.

  I don't really know why it didn't work without IDs, but most likely you have conflicting css overwriting that specific part of your code. I'm guessing it is a specificity problem, an inheritance problem or a order in code problem. You'll have to debug to find the culprit.

  Tips on debugging tricky CSS:

  • In a browser's Inspector, when you select an element you can go to Computed to see which selector is currently active on that element, perhaps that'll help you identify what's overriding your code.
  • Use !important proactively to debug. While one shouldn't use !important to fix problems, one can use it to check if a selector works at all. If you see a change when you slap on !important, you know the selector works, but something else is overriding your code, somewhere. If you can't see any change, then the selector isn't working on the element.
  • Try chaining a class to raise its specificity. You do so by going .class.class.class. This is functionally similar to .something.other-thing.third-thing, but you only need one class in your html to use it. This raises the specificity from 010 to 030, if you use IDs the specificity will be 100 instead.

  One last tip: line-height doesn't require a unit, but will work as a relative measure. Meaning, you can write line-height: 1.5; and you'll have a line height of 1.5 regardless of what the font size is, but if you specifically set a unit, say line-height: 1.9rem;, the line height will always be that size even if you change to a larger font size later/elsewhere.

  Marked as helpful

  0

  ---

  

  Kathryn•320

  @KathrynDavies123Posted almost 2 years ago

  @kkalvaa Thanks! Didn't think about the fact I can just use the class in css :) will implement all of this and reupload. Cheers

  0
• 

  Kristian Kalvå•360

  @kkalvaaPosted almost 2 years ago

  Some comments on your CSS:

  It is generally better to use class selectors over id selectors as classes have a much lower specificity and is reusable. So instead of #billamount you should use .billamount. classes can still be single use.

  About your naming of classes: The common CSS naming structure is use hyphens to separate words for readability: .billamount becomes .bill-amount, #numberofpeople becomes .number-of-people, etc.

  "Magic numbers": I noticed that you've used a lot of exact pixel sizes for things, instead of relative sizes, such as ems or rems. Most of the time it is better to not use an exact number and instead use relative sizes. Sources:

  • https://css-tricks.com/magic-numbers-in-css/
  • https://csswizardry.com/2012/11/code-smells-in-css/#magic-numbers

  You also have a lot of repetition where different classes have the same content. Example: *#billamount { background-image: url("../images/icon-dollar.svg"); background-repeat: no-repeat; background-position: center; background-position-x: 20px; }

  #numberofpeople { background-image: url("../images/icon-person.svg"); background-repeat: no-repeat; background-position: center; background-position-x: 20px; }*

  The only difference here is the background image URL. It would be better to use one common class for both and then one specific one for each where you add the background image. Example:

  *.icon-button { background-repeat: no-repeat; background-position: center; background-position-x: 20px; }

  .icon-button--dollar { background-image: url("../images/icon-dollar.svg"); }

  .icon-button--people { background-image: url("../images/icon-person.svg"); }*

  while you don't gain anything in terms of code length, you do gain maintainability. This way if you need to update something on .icon-button you only update it once instead of multiple times, which leads to lower chance of errors/typos.

  DON'T USE !important !important ruins specificity and leads to a lot of problems in itself. Most of the time you can fix the issue with a refactor.

  0

  ---

  

  Kathryn•320

  @KathrynDavies123Posted almost 2 years ago

  @kkalvaa Hi, thanks for the tips.

  The reason I used IDs for the number inputs and not classes was because they had labels, and to be semantically correct I thought the label "for" property had to be assigned to an ID, since they are unique. That is also the reason I had to use !important to get the error message border through, because the class didn't overwrite the ID as it had higher specificity.

  How would you solve this?

  Thanks,

  Kathryn

  0

  

  Kathryn•320

  @KathrynDavies123Posted almost 2 years ago

  @kkalvaa Also, using one common class for the background properties didn't work, hence why I had to put them in with the id selectors separately. I already had a common class called "largeinput", but when I add the background repeat etc. to that it doesn't work.

  0