Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add color-adjust property example #1008

Merged
merged 4 commits into from
Jul 9, 2018

Conversation

a2sheppy
Copy link
Contributor

Adds color-adjust interactive example.

Copy link

@schalkneethling schalkneethling left a comment

Choose a reason for hiding this comment

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

Thanks for the example @a2sheppy, took a little reading up to realise how this works ;) Two nits, other than that r+w/c

@@ -0,0 +1,11 @@
#example-element {
padding: 1em;
font-size: 1.5em;

Choose a reason for hiding this comment

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

Super Nit: Code style: Prefer rem over em for font sizes.

padding: 1em;
font-size: 1.5em;
background-color: black;
background-image: linear-gradient(rgba(0, 0, 180, 0.5), rgba(70, 140, 220, 0.5));

Choose a reason for hiding this comment

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

Nit: Code style: Omit 0 from fractional values

@a2sheppy
Copy link
Contributor Author

a2sheppy commented Jul 6, 2018

Remove the leading "0" from fractional numbers? That's... strange. Leaving out the leading 0 makes the number harder and violates every style guide I've ever seen.

@schalkneethling
Copy link

@a2sheppy This has been part of the Kuma stylelint rules for a long time:
https://github.com/mozilla/kuma/blob/master/.stylelintrc#L54

Here is the documentation from stylelint:
https://stylelint.io/user-guide/rules/number-leading-zero/

It is also part of sass-lint:
https://github.com/brigade/scss-lint/blob/master/lib/scss_lint/linter/README.md#leadingzero

And also mentioned in Codestyle.io's documentation:
http://codestyle.co/Guidelines/css#Syntax

I also generally find it to be the format I most encounter.

@a2sheppy
Copy link
Contributor Author

a2sheppy commented Jul 9, 2018

It's still weird; I've honestly never seen such a thing. I'll make the change, but I think it badly damages legibility of the code.

Changed from em to rem units and removed the leading
"0" from fractional values (boy, that still looks so
*weird** to me... but rules is rules!). :)
@chrisdavidmills
Copy link
Contributor

I've seen both in fairly equal measure, FWIW. I think I prefer "with leading 0s", for similar reasons to those sheppy brings up, but I can live with it.

@schalkneethling
Copy link

@a2sheppy @chrisdavidmills Yeah, this is one of those points where there is often contention ;) As we have followed this guideline for the rest of this codebase, and have also been following it on Kuma since before my time, I felt that we should stick with it.

Copy link

@schalkneethling schalkneethling left a comment

Choose a reason for hiding this comment

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

r+ Thanks @a2sheppy

@schalkneethling schalkneethling merged commit e1a34cb into mdn:master Jul 9, 2018
wbamberg pushed a commit to wbamberg/interactive-examples that referenced this pull request Jul 12, 2018
* upstream/master: (33 commits)
  Add HTML example for <a> (issue mdn#1014) (mdn#1022)
  fix(tabbed): load fonts as part of the editor css as @font-face does not work inside shadow dom (mdn#1015)
  html/input: Lowercase attributes (mdn#1024)
  Add color-adjust property example (mdn#1008)
  chore(deps): update dependency eslint to v5.1.0 (mdn#1023)
  chore(community): add @goodwin64 as contributor (mdn#1021)
  chore(community): add @dagolinuxoid as contributor (mdn#1020)
  chore(community): add @arai-a as contributor (mdn#1019)
  chore(community): add @ro-ka as contributor (mdn#1018)
  Issue mdn#1013 <style> elements in HTML editor break editor's render method (mdn#1017)
  chore(deps): update jest monorepo to v23.3.0 (mdn#1012)
  Issue#946 open links in new tab (mdn#1009)
  Send metric only during loading event (mdn#1011)
  fix(performance): only send loading mark inside loading event (mdn#1010)
  chore(deps): update dependency prettier to v1.13.7 (mdn#1006)
  fix(address): add name to the provided address (mdn#1004)
  chore(community): add @Arkangus as contributor (mdn#1002)
  Correct "expected output" mistake (mdn#1000)
  fix(performance): send post to kuma for custom ie-load-event-end metric (mdn#1001)
  chore(deps): update jest monorepo to v23.2.0 (mdn#993)
  ...
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants