-
Notifications
You must be signed in to change notification settings - Fork 4
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/1545 (part 1) initial addition of files from miles repo for live comms website #1555
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we committing the whole USWDS library here? @reitermb Can't we just link to the source in the relevant html files?
I only point that out because this PR introduces a nontrivial amount of potentially unneeded files and code |
|
Codecov Report
@@ Coverage Diff @@
## raft-tdp-main #1555 +/- ##
=================================================
- Coverage 97.58% 97.55% -0.03%
=================================================
Files 80 80
Lines 1901 1884 -17
Branches 249 245 -4
=================================================
- Hits 1855 1838 -17
Misses 22 22
Partials 24 24
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
I can't find an official CDN, and all their documentation suggests the best way to include it is through npm. How do you suggest we do this? |
A non-official CDN will more than suffice for these purposes, in my opinion https://www.jsdelivr.com/package/npm/uswds |
|
For reference, there is a conversation in place about an official CDN |
Maybe we can also pin point exactly which of these assets we need, and which were just added as boilerplate when you created the github website? @reitermb cc @jorgegonzalez |
Is that really worth the effort over just using the CDN? |
Is this mainly about all the assets (e.g. icons) and having both minified and non versions of css, JavaScript, etc...? I'm open to suggestions on hosting those anywhere (and pruning out unneeded files for that matter) so long as it doesn't impact our ability to make tweaks to them/supplement them/add to them. |
sort of a side thing, this is 500+ files, the CDN gets rid of 3 or 4 of them. A lot are images, fonts, etc. I am wondering if we can get rid of some of those. |
Hmm I thought it could have gotten rid of more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving this as including all this in the repo seems to be the simplest solution at this time
@jorgegonzalez from the looks of it, there are a bunch of style files that are important to USWDS that I don't think would get included with the js files. I think there is more to it than that. |
Those CDNs don't just serve js though |
I managed to get rid of 400 files, the whole source code seemed to be incldued and it wasn't needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏾
Summary of Changes
Since the live comms site is over 500 files we already know we want, to make the deployment adjustments easier to review this PR just introduces the files as they already were.
How to Test
List the steps to test the PR
These steps are generic, please adjust as necessary.
Deliverable 1: Accepted Features
As Product Owner, @lfrohlich will decide if ACs are met.
Deliverable 2: Tested Code
Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: Context