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

feat: added ca-gov custom css and updated header #9

Merged
merged 5 commits into from
Jan 28, 2025
Merged

feat: added ca-gov custom css and updated header #9

merged 5 commits into from
Jan 28, 2025

Conversation

kkoryaka
Copy link
Contributor

No description provided.

@kkoryaka kkoryaka requested a review from thekaveman January 27, 2025 23:38
Copy link

github-actions bot commented Jan 27, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  web
  settings.py 43-45
  wsgi.py
  web/core
  models.py
  web/oauth
  admin.py
  client.py 9, 16, 22, 44
  models.py
  session.py 3-29
  views.py 7-9, 16-20, 26-37, 42-47, 75-110, 143-148, 159-160
  web/oauth/migrations
  0001_initial.py 4-5
  web/vital_records
  urls.py 2, 7
Project Total  

This report was generated by python-coverage-comment-action

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

@kkoryaka, is this a copy of a stylesheet from another website? It seems like there are a lot of style rules in here that aren't used / don't target anything in this HTML.

E.g. this site doesn't have a div.header-banner and so this entire rule is not applied

main {
  > div.header-banner {
    overflow: hidden;
    background-image: url('data:image/svg+xml,<svg version="1.1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1440 498"><polygon fill="%23035D85" points="598 -1 -1 -1 -1 243 -1 244 -1 497 98 497 98 497 840 497"/><polygon fill="%23035376" points="1204 -1 598 -1 840 497 949 497 1440 497 1440 -1"/></svg>');
    background-color: var(--color-p2, #046b99);
    background-position: center !important;
    background-size: cover !important;
    display: block;
    position: relative;
    @media (width <= 991px) {
      > div.row {
        flex-direction: column;
      }
    }
  }
}

There are other examples like this in the file.

I have two thoughts:

  1. Would it make sense to just link to a stylesheet on another site instead of copying this in? If this is really just a copy of another stylesheet with no customizations, let's just reference it directly.
  2. Or would it make more sense to clean up this stylesheet to only those customizations that are needed for this site?

@kkoryaka
Copy link
Contributor Author

@kkoryaka, is this a copy of a stylesheet from another website? It seems like there are a lot of style rules in here that aren't used / don't target anything in this HTML.

E.g. this site doesn't have a div.header-banner and so this entire rule is not applied

main {
  > div.header-banner {
    overflow: hidden;
    background-image: url('data:image/svg+xml,<svg version="1.1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1440 498"><polygon fill="%23035D85" points="598 -1 -1 -1 -1 243 -1 244 -1 497 98 497 98 497 840 497"/><polygon fill="%23035376" points="1204 -1 598 -1 840 497 949 497 1440 497 1440 -1"/></svg>');
    background-color: var(--color-p2, #046b99);
    background-position: center !important;
    background-size: cover !important;
    display: block;
    position: relative;
    @media (width <= 991px) {
      > div.row {
        flex-direction: column;
      }
    }
  }
}

There are other examples like this in the file.

I have two thoughts:

  1. Would it make sense to just link to a stylesheet on another site instead of copying this in? If this is really just a copy of another stylesheet with no customizations, let's just reference it directly.
  2. Or would it make more sense to clean up this stylesheet to only those customizations that are needed for this site?

@thekaveman you are right. I cleaned it and left only those styles that currently used and couple of utilities

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

@kkoryaka Sorry to be a stickler. I found some more rules that appear to be unused given our HTML markup.

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

Looks great @kkoryaka, thank you!

@kkoryaka kkoryaka merged commit 091acd4 into main Jan 28, 2025
2 checks passed
@kkoryaka kkoryaka deleted the ca-gov branch January 28, 2025 23:28
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.

2 participants