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

Audit _shame.scss #992

Merged
merged 5 commits into from
Nov 17, 2023
Merged

Audit _shame.scss #992

merged 5 commits into from
Nov 17, 2023

Conversation

ataker
Copy link
Contributor

@ataker ataker commented Nov 15, 2023

Description

Complete an audit of _shame.scss. Remove unused code and add comments to code remaining about its use.

Closes Audit _shame.scss

Testing done

Almost none, only removed code that had no use in vets-website or content-build. Some I ran to double check an edge case, but mostly untested.

Acceptance criteria

  • Unused code is removed

Definition of done

  • Changes have been tested in vets-website
  • Changes have been tested in IE11, if applicable
  • Documentation has been updated, if applicable
  • A link has been provided to the originating GitHub issue (or connected to it via ZenHub)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs

@ataker ataker requested a review from a team as a code owner November 15, 2023 16:09
Copy link
Contributor

@jamigibbs jamigibbs left a comment

Choose a reason for hiding this comment

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

  • Is this bit only for IE support? If so, we might be able to remove it too since we no longer support that browser:

    // force use of png; IE does not support SVG
    select {
    background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAcCAYAAABh2p9gAAAAAXNSR0IArs4c6QAAAPFJREFUSA3tlr0OAUEUhZdCRSQSBVFsQuNtPCS9t7CFRqNAFAoPoeE7m72FiTHWXp2THPN3z7eJ3Z3ZLEtrTsmxsvqNNCN9xffK6mvuK+WkLthg1mpOa7U0ofqMDRK2WlPNRxpRdcAhJByrRrVvNWR1j8NwbKxaZV5qwOwOx8KxeWWUfVKf0RbHQql5ZcUo1eN3g1Oh1LoYYmUrB5hdbNkGNhXVSbMWoDFe4E5D6I38uiHjH+cf+MlNKQDbc9S0LfQcnhzvVcnqAvR69cQq5bo5GNR1+zKo6wZrUNcjwKCuh5RBczpux6hBXQ96g9b6FHkA0+zeRS2oYfAAAAAASUVORK5CYII=);
    }

  • And if we're creating a release after this, package.json version will need to be updated.

Copy link
Contributor

@micahchiang micahchiang left a comment

Choose a reason for hiding this comment

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

looks good, thanks for taking care of this @ataker! I just left one comment about the edit-checkbox styles.

packages/formation/sass/_shame.scss Show resolved Hide resolved
@@ -1,6 +1,6 @@
{
"name": "@department-of-veterans-affairs/formation",
"version": "7.1.0-color-beta",
"version": "8.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bumped major version since removing could be a breaking change. Let me know if you disagree

@ataker ataker merged commit 0c61fb8 into master Nov 17, 2023
6 checks passed
@ataker ataker deleted the dst2044-audit-shame-scss branch November 17, 2023 16:49
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.

Audit _shame.scss stylesheet in Formation
4 participants