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

Upgrade to Tailwindcss 3 and use via play cdn #36

Merged
merged 26 commits into from
Jan 13, 2022
Merged

Upgrade to Tailwindcss 3 and use via play cdn #36

merged 26 commits into from
Jan 13, 2022

Conversation

zampino
Copy link
Collaborator

@zampino zampino commented Dec 16, 2021

Upgrade tailwind to version 3 and switch to using the tailwind play cdn.

Also copy the viewer stylesheet in the resource path so it's available with the default deps (the viewers module is only a dev-time dependency).

  • use tailwind from CDN
  • copy viewer.css from viewers in classpath when bb dev starts locally

@zampino zampino marked this pull request as ready for review December 16, 2021 17:56
@zampino zampino marked this pull request as draft December 16, 2021 18:10
deps.edn Outdated

;; needed for CSS jit pass
babashka/process {:mvn/version "0.0.2"}
io.github.nextjournal/cas {:git/url "git@github.com:nextjournal/cas"
Copy link
Collaborator Author

@zampino zampino Dec 16, 2021

Choose a reason for hiding this comment

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

Damn, this is a private repo :-( It can't be added to main deps.

Copy link
Member

Choose a reason for hiding this comment

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

Don’t understand why we would need that. Think we should just have the css locally, and uploading it should be an independent concern, same as we don’t upload the html files.

(when-not (fs/exists? (fs/parent "build/viewer.js"))
(fs/create-dirs (fs/parent "build/viewer.js")))
(spit "build/viewer.js" ;; slurp latest build from CAS as configured in n.clerk.view
(slurp (-> (io/file "src/nextjournal/clerk/view.clj")
Copy link
Member

Choose a reason for hiding this comment

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

There’s a much easier way to get to this, isn’t there? 😜

this assumes push rights to nextjournal CAS and a node environment
capable of running tailwindcss. Also downloads js build from CAS as it's
needed for tailwind to purge unused classes.
@philippamarkovics
Copy link
Member

There seems to be styles missing when running bb dev and looking at the devcards. Am I holding it wrong?

CleanShot 2021-12-17 at 11 33 44@2x

@philippamarkovics
Copy link
Member

I tested static builds with and without purge and the results look good. @mk I see what you mean with the waiting times now when purge-css? is enabled. I talked with @kommen about this but we both don’t see a way right now to make this more performant.

@mk
Copy link
Member

mk commented Dec 17, 2021

Devcards look good here. yarn install to the rescue?
CleanShot 2021-12-17 at 12 03 13@2x

@mk mk mentioned this pull request Dec 21, 2021
@mk mk changed the title Optional Tailwind JIT Upgrade to Tailwindcss 3 via play cdn & enable optional css purging Dec 21, 2021
@zampino
Copy link
Collaborator Author

zampino commented Jan 3, 2022

Just a side note on

Consolidate into one tailwind config

in principle, now that we use tailwind from CDN, we'll only need a config in the invocation

npx tailwindcss --input stylesheets/app.css --config tailwind-jit.config.js --output public/build/clerk.css -m

within build-static-app!. We can move such a config into resources and interpolate its absolute path in the command. Same could happen for the --input I guess, after @joe-loco refactoring of viewer.css.

philippamarkovics and others added 2 commits January 10, 2022 14:57
use stdin in tailwind cli for reading input from resource. Cleanup 
unused bb tasks.
@zampino zampino changed the title Upgrade to Tailwindcss 3 via play cdn & enable optional css purging Upgrade to Tailwindcss 3 and use via play cdn Jan 13, 2022
if we use the same name the update will never take place since 
`viwer-css-path` will point
at clerk's own resource.
@zampino zampino force-pushed the tailwind-jit branch 2 times, most recently from 5104436 to 659b3e6 Compare January 13, 2022 12:59
@zampino
Copy link
Collaborator Author

zampino commented Jan 13, 2022

This .not-prose stuff is biting us here a bit (but not so crucial), if we use clerk/html to render hiccup which actually contains prose (as build from markdown ->hiccup) the container is assigned a not-prose class and this now happens:

CleanShot 2022-01-13 at 14 27 32@2x

while it was like this before:

CleanShot 2022-01-13 at 14 27 47@2x

@zampino
Copy link
Collaborator Author

zampino commented Jan 13, 2022

Some styles are off wrt to main from the tailwind rework but we can maybe fix them in later steps? (left demo on main / right against this branch)

CleanShot 2022-01-13 at 14 49 22@2xCleanShot 2022-01-13 at 14 49 27@2x

Maybe I can try to fix results to clerk/code which now get no background.

avoid repeating not-prose / fix result inner viewer class
@zampino zampino marked this pull request as ready for review January 13, 2022 15:04
@mk mk merged commit 68fda01 into main Jan 13, 2022
@mk mk deleted the tailwind-jit branch January 13, 2022 20:30
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.

3 participants