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

Make queso-ified templates the default #73

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

mandicai
Copy link
Contributor

@mandicai mandicai commented Jan 29, 2021

Todos:

  • Have a meeting with design to audit some of our graphic styles. Before moving our templates to fully use queso, we should make sure our current graphic styles are what we want. Once we do the audit, we can figure out which custom styles we want to keep and which to start getting from queso.

Changed

  • templates/__common__/_package.json - bump version of queso-ui.
  • templates/feature/app/index.html - changed ad inputs, added comments, set variable for feature data
  • templates/feature/app/templates/macros/processors.html - added CSS classes to list component, fixed ad component inputs (to allow us to process ads fetched in the ArchieML text)

Removed

  • Old common typography.scss and prose.html, feature index.html, main.scss and processors.html files were replaced by the queso-ui versions.

To test

  • Checkout this branch in data-visuals-create
  • Go to another folder and generate a feature by running path/to/data-visuals-create/bin/data-visuals-create feature test-feature
  • npm run data:fetch
  • npm run serve
  • Make sure everything looks OK on desktop and mobile locally

Default feature ArchieML text (what is fetched when you run npm run data:fetch)

@ashley-hebler
Copy link
Member

Oh this is neat! I might also suggest bumping up to the latest version to get the new helper classes listed there


// Third-party libraries
// third-party libraries
Copy link
Member

Choose a reason for hiding this comment

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

Fortunately, we can now get rid of the sass-mq import here (lines 16 and 17). We import it through line 2, which links back to this file

Copy link
Member

@ashley-hebler ashley-hebler left a comment

Choose a reason for hiding this comment

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

This is looking great and I'm so excited to see the shared styles as a default. I introduced a bug when I didn't include the unused CSS task in the parser so I went ahead and committed that fix with f0a5111 since it was my fault in the first place. We didn't notice it before because graphics used the old location for CSS (not in the /min folder.)

Feel free to ignore this, but I noticed that the grays used in the graphics are a little light. Their contrast is being flagged in some of my accessibility testing tools. Maybe a question to take up with the rest of the team though.

Screen Shot 2021-03-17 at 9 25 30 AM

Thanks for doing this!

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.

None yet

2 participants