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

Discovery on best way to import USWDS images and fonts #66

Open
annekainicUSDS opened this issue Jun 6, 2018 · 15 comments
Open

Discovery on best way to import USWDS images and fonts #66

annekainicUSDS opened this issue Jun 6, 2018 · 15 comments
Labels
[practice] engineering Engineering related work [type] debt Tech debt, refactors, maintenance issues

Comments

@annekainicUSDS
Copy link
Contributor

Right now, Vets.gov and the VA design system imports the images and fonts from USWDS directly, by coping all of the files into an assets directory. This is what I've done temporarily for USWDS as well. There may be a better solution though; look into webpack handling this potentially.

@annekainicUSDS annekainicUSDS added [practice] engineering Engineering related work [type] debt Tech debt, refactors, maintenance issues labels Jun 6, 2018
@jcmeloni-usds
Copy link
Contributor

So (ignore if this is too much of a tangent) is uswds in the package.json not really fully useful if we have to manually copy things anyway? Or is it that the package puts things in weird places and it's too much work to remap where those places are so we just manually copy them over?

@annekainicUSDS
Copy link
Contributor Author

These are good questions that I need to answer when looking into this ticket more :) For expediency I just copied what vets.gov was doing to get the starter-app working, but definitely want to find a better long-term solution

@jcmeloni-usds
Copy link
Contributor

haha oops, my bad?

@annekainicUSDS
Copy link
Contributor Author

That's okay! Wasn't clear from the description

@annekainicUSDS
Copy link
Contributor Author

Another problem to look into here: One of the fonts that is used in our forms is imported in a font faces file in the VA design system: https://github.com/department-of-veterans-affairs/design-system/blob/master/src/sass/base/_b-font-faces.scss. This font is copies into vets-website under the assets/ directory, and then this directory is redirected to / in the build script: https://github.com/department-of-veterans-affairs/vets-website/blob/master/script/build.js#L526. When the stylesheet looks for the font, it will look at /font/.... This is a problem because in us-forms-system we're not redirecting our fonts to the base path, so the url that is specified in the font-faces file from VA design can't be found.

As a temporary measure, I've created a different version of the font faces file that changes the path to be relative to the directory it's in, and then copied those fonts into lib/fonts, but would like to find a better long-term solution for this. PR that makes these changes is here: #81

@jcmeloni-usds
Copy link
Contributor

@annekainicUSDS @ju-liem do we need a new ticket to reference the current state, and close this since a bunch of work has happened on it? just want to keep ducks in a row. 🦆 🦆 🦆

@annekainicUSDS
Copy link
Contributor Author

I'd like to keep this ticket open since it's slightly different from the work I've been doing to refactor the scss (haven't specifically addressed USWDS issues yet, have been focusing on pulling out VA design system styles).

@jcmeloni-usds
Copy link
Contributor

@annekainicUSDS @fatima-gov Is there a tl;dr on what's happened with this or is the reference to #152 the most accurate? Thanks!

@annekainicUSDS
Copy link
Contributor Author

I haven't looked at this ticket since I last commented, so no new information from me :) it's not high priority as far as I see it. Basically I believe there to be a better way to import USWDS images and fonts from how we are currently doing it, but what we have now works. Perhaps this is something we can revisit with the 2.0 upgrade?

@jcmeloni-usds
Copy link
Contributor

Sounds good!

@dmethvin-gov
Copy link
Contributor

As I've been looking at the issues, this ticket seems increasingly important as far as wider adoption is concerned.

The current top-level SCSS file has VA-specific global design overrides plus specific ones as well. I'm not sure of the impact of removing them, for example whether the widgets or page layout that USFS has depends on the VA overrides. If so, that would need to change?

I think it would be best if our USFS clients got a non-modified USWDS setup so that we could just point them to the USWDS documentation and not need to document a set of changes that they may not want. However, the way CSS is currently processed uses styles.scss to modify SCSS variables in the middle of the includes. Can we just override styles for the things we need for specific widgets?

If we are breaking out the navigation into the starter app(s) it seems like those styles need to be in those apps and not in USFS. Would it be enough to have the starter app just override the cascade in a plain CSS file or will we require them to process the USWDS SCSS files and include their overrides via includes at strategic points the way we do with styles.scss?

Although it's not something to tackle until we've decided on other things, I've noticed that the base styles.css from USWDS is only 104KB but our final file is about 180KB. Not sure why/how we added so much to the size.

It would be worth surveying other React projects (libraries in particular) to see how they're addressing this issue, this article mentions a few. I noticed the e-QIP prototype uses separate CSS files for each widget, which would allow only pulling in the required CSS and could save quite a bit of file size. To do that we'd need to actually know which widgets are in use. The current formConfig doesn't tell us that since we pass in imported references to JS modules. We could require them to add a matching import 'dateWidget.css' but that seems error prone.

@dmethvin-gov
Copy link
Contributor

Well, I get the impression there is absolutely no agreement on how to incorporate or share CSS in a React library. Here is what I found various libraries are using:

@dmethvin-gov
Copy link
Contributor

@dmethvin-gov
Copy link
Contributor

I am assuming that most users will want to build more visuals (intro pages, thank-you pages, instructions, etc.) that match USWDS styles, and they often will want to add or modify styles. (For example, vets.gov did both of these things.)

With that in mind we need to give the app control over how styles are built and included. It could work like this:

  • The app declares USWDS as a dependency
  • We document which parts of USWDS are required for the USFS built-in widgets and definitions, and define our own additional styles (if needed).
  • The app has something similar to the current styles.scss that pulls together the USWDS, USFS, and app overrides into a single compiled CSS file.

Starter app(s) can show how to do this.

Are there any hard USWDS dependencies we actually need inside USFS? We're using jsdom for testing which doesn't even have a layout engine, so we can't be doing any checks for visual rendering. I'm wondering if we should even call it a peerDependency for the library as a whole, but if we split off the widgets it would be nice to have some sort of basic rendering integration tests to ensure styles were being applied properly.

@easherma
Copy link

I think this is a valuable conversation, and I'll just stub this here to say in Austin we are cruising into handling this situation: https://github.com/cityofaustin/officer-complaint-form

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[practice] engineering Engineering related work [type] debt Tech debt, refactors, maintenance issues
Projects
None yet
Development

No branches or pull requests

4 participants