Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Replace deprecated darken with color.adjust for Sass compliance #634

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

uditjainstjis
Copy link

Summary

Changes/explanation

  • darken function was replaced with color.adjust which is supported
  • '{' bracket was added at the missing place
  • Ran the command: sass /css/ion_icon_customization.scss /css/ion_icon_customization.css to generate the compiled css file, which was necessary for "@use keyword" in @use "sass:color" which was used to use color thing to replaced darken the main thing.

Testing

  • Build the site locally with jekyll serve and verify image and list rendering.
  • Check bullet points and image scaling on mobile and desktop views.
  • Ensured now no error is present in the terminal and is clear.

Copy link
Contributor

@quozl quozl left a comment

Choose a reason for hiding this comment

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

  • don't write so much in the pull request alone, what you said should be in the commit message,
  • rewrite your commit message and force push, see our guidance for a checklist,
    • add the problem to the commit message, as the GitHub issue will be lost and the commit message kept,
    • add the solution to the commit message, as the GitHub pull request will be lost and the commit message kept,
  • do not make changes unrelated to your stated intent, be more careful, use interactive add of git if necessary to avoid any changes made by your software tools,

Please fix these things in your existing branch, squash your commits, and then force push to update the pull request.

Copy link
Contributor

Choose a reason for hiding this comment

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

Generated files should not be committed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Generated files should not be committed.


.ion-sticky {
top: 50%;
transform: translateY(-50%);
-webkit-transform: translateY(-50%);
Copy link
Contributor

Choose a reason for hiding this comment

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

Unexplained change.

@@ -72,10 +65,6 @@ $medium-background-hover: darken($medium-background, $darken-percent);
display: block;
clear: both;
transition: padding 0.1s ease-out;
-webkit-transition: padding 0.1s ease-out;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unexplained change.

@@ -106,7 +95,7 @@ $medium-background-hover: darken($medium-background, $darken-percent);
background: $github-background;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary change to whitespace.

&instagram {
// Instagram uses background color
Copy link
Contributor

Choose a reason for hiding this comment

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

Unexpected removal of comment.

@@ -288,8 +276,8 @@ $medium-background-hover: darken($medium-background, $darken-percent);
&medium {
color: $medium-color;
font-size: 3rem;
height: 3rem;
width: 3rem;
height: 3rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unexpected change.

@@ -299,4 +287,4 @@ $medium-background-hover: darken($medium-background, $darken-percent);
}
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Removal of newline at end of file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecated function darken being used, causing warning
2 participants