Skip to content

Commit

Permalink
[REFACTOR] Clearly distinguish prod and development css (#4148)
Browse files Browse the repository at this point in the history
After the discussion in #4102 and #4130, I will try to clean up the mess I made!

Here is the new situation:

* We now have two different scripts, `generate-prod-css` and `generate-development-css`.
* These generate into two different files, `generated.full.css` and `generated.css`. The `generated.css` file is `gitignore`d.
* Depending on whether we are in debug mode or not, the right file is referenced in the HTML.

Hopefully, this will get rid of all the confusion surrounding the purpose of these files!

@TiBiBa, @reedspool, what do you think?
  • Loading branch information
rix0rrr authored Mar 21, 2023
1 parent 052ca60 commit b2ab78c
Show file tree
Hide file tree
Showing 15 changed files with 32,040 additions and 4,234 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,6 @@ grammars-Total/*.lark
.dst
*.mp4
*.pid

# Prod css doesn't have to be checked in, only leads to merge conflicts
./static/css/generated.css
68 changes: 43 additions & 25 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
Helping build Hedy
------------

We would be grateful if you want to help make Hedy better!
We would be grateful if you want to help make Hedy better!

**How to get in touch with the team**
Hedy is built with a team of people, and while we love people to help out, we do ask that you let us know that you want to work on something before doing so,
(either on GitHub or on Discord) so we don't get overwhelmed with small PRs to review for issues that don't have a lot of priority.
(either on GitHub or on Discord) so we don't get overwhelmed with small PRs to review for issues that don't have a lot of priority.

Our main channel of communication is our [Discord](https://discord.gg/8yY7dEme9r), so please join us there and let us know that you want to help out.
We also have bi-weekly contributors meetings on Tuesdays at 7:30 CET (dates and link are in Discord) where we discuss larger issues and plans.
Expand All @@ -31,7 +31,7 @@ If you are interested in working on topics related to an open discussion, please
Contributing to Hedy
------------

The easiest way to get a working development environment to work on Hedy is through [Github Codespaces](https://github.com/features/codespaces).
The easiest way to get a working development environment to work on Hedy is through [Github Codespaces](https://github.com/features/codespaces).

[![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://github.com/codespaces/new?machine=basicLinux32gb&repo=226863521&ref=main&location=WestEurope)

Expand All @@ -46,7 +46,7 @@ Run Hedy on your machine

VS Code has a great Dev Containers extension that allows you to connect the IDE to a development (docker) container. More info can be found on https://code.visualstudio.com/docs/devcontainers/containers

After opening this repo in VS Code, they will ask whether you want to open this folder in a container. Do this and you will have a working environment in which you can develop Hedy.
After opening this repo in VS Code, they will ask whether you want to open this folder in a container. Do this and you will have a working environment in which you can develop Hedy.


**Local installation**
Expand Down Expand Up @@ -103,7 +103,7 @@ $ npx cypress run
```
For the tests to be truly reproducible, you need to clean the database first and put our tests data there, for that you can need to run the `feed_dev_database.sh` script.

You can also run a particular set of tests on the command line with the following command:
You can also run a particular set of tests on the command line with the following command:

```bash
npx cypress run --spec "[path to test(s)]"
Expand All @@ -127,8 +127,8 @@ Do note a few things:
* For the same reason, set your app to English
* ensure the `ADMIN_USER` environment variable is set to `admin` before starting the app. e.g.
``` bash
$ . ./.env/bin/activate
(.env)$ export ADMIN_USER=admin
$ . ./.env/bin/activate
(.env)$ export ADMIN_USER=admin
(.env)$ python app.py
```
If you want to connect Cypress to the online dashboard, use:
Expand All @@ -140,7 +140,7 @@ npx cypress run --record --key <key here>
To check the front end test coverage, you can run the script:

```bash
./tests/get-code-coverage
./tests/get-code-coverage
```

And then go open the `index.html` file located in `tests/coverage/lcov-report`, for more information about how this all works you can go (here)[https://docs.cypress.io/guides/tooling/code-coverage]
Expand Down Expand Up @@ -204,29 +204,47 @@ Make sure to reload your browser (and work in incognito mode) to see the changes
These files are also automatically generated on deploy, so don't worry if you forget to generate them.

## Working on the web front-end in Tailwind
All the styling in our front-end HTML templates is done using the Tailwind library.
This library has generated classes for styling which we can apply to HTML elements.
To make sure you have access to all possible styling classes, generate the development CSS file:

All the styling in our front-end HTML templates is done using the [Tailwind
library](https://tailwindcss.com/docs/installation). This library has generated
classes for styling which we can apply to HTML elements.

You normally do not need to think about this. During development, a multi-megabyte
CSS file will be served that contains most classes. During deployment, a minimized CSS
file is automatically produced.

You may need to regenerate the development CSS file if you want to do one of the following things:

* Use a *conditional* Tailwind class (for example, a class that starts with `hover:`).
Write the class in the HTML, then regenerate the CSS.
* Add custom classes to `styles.css`.

Run the following command to regenerate the development CSS file:

```bash
$ ./build-tools/heroku/tailwind/generate-development-css
```
When merging we want to keep the CSS file as small as possible for performance reasons.
Tailwind has a built-in `purge` option to only generate CSS for classes that are actually being used.
Please run the following command so Tailwind only generates used classes:
```bash
$ ./build-tools/heroku/tailwind/generate-css
```

For all possible styling classes and more, take a look at their [website](https://tailwindcss.com).
If you want to combine different Tailwind classes into one class or one element, we can do this in the `/build-tool/heroku/tailwind/styles.css` file.
By using the `@apply` attribute we can assign classes to other styling. For example, we styled the `<h1>` element with multiple Tailwind classes like this:

If you want to combine different Tailwind classes into one class or one element,
we can do this in the `/build-tool/heroku/tailwind/styles.css` file. By using
the `@apply` attribute we can assign classes to other styling. For example, we
styled the `<h1>` element with multiple Tailwind classes like this:

```css
h1 {
@apply font-extralight text-4xl;
}
```
If you want to use styling that is not available in the Tailwind library this can be added to the `static/css/additional.css` file.
But please, try to use the Tailwind classes as much as possible as these are optimized and keep our code base consistent and readable.
Also, please refrain from using inline CSS styling, as this makes the templates hard to read, maintain and alter.

If you want to use styling without running a Tailwind build and without using
Tailwind classes, add it to `static/css/additional.css` file. But please, try to
use the Tailwind classes as much as possible as these are optimized and keep our
code base consistent and readable.

Also, please refrain from using inline CSS styling, as this makes the templates
hard to read, maintain and alter.

## Working with translations

Expand Down Expand Up @@ -307,7 +325,7 @@ docker build -t hedy .
and then you can run the docker container with:

```bash
docker run -it --rm -p 8080:8080 --mount type=bind,source="$(pwd)",target=/app --name hedy hedy
docker run -it --rm -p 8080:8080 --mount type=bind,source="$(pwd)",target=/app --name hedy hedy
```

After that, you can access bash inside the container with:
Expand All @@ -317,7 +335,7 @@ docker exec -it hedy bash

## Testing Admin facing features locally

For some things like making classes you need a teacher's account which you might want to test locally.
For some things like making classes you need a teacher's account which you might want to test locally.
For that you can use the account `teacher1` which is stored in the local database.
If you want to try Admin features locally (for example, marking accounts as teacher or updating tags) you have to run Hedy with the environment variable `ADMIN_USER` set to your username, e.g. `ADMIN_USER=teacher1`. It works a bit differently in each IDE, this is what it looks like for PyCharm:
Expand All @@ -326,7 +344,7 @@ If you want to try Admin features locally (for example, marking accounts as teac
## What happens when I make a PR?
When you create a pull request, someone will take a look and see whether all is in order. It really helps if you let us know how to test the PR (this is also documented in the PR template) and if you yourself make sure all is in order by running the tests locally.
When you create a pull request, someone will take a look and see whether all is in order. It really helps if you let us know how to test the PR (this is also documented in the PR template) and if you yourself make sure all is in order by running the tests locally.
If the PR is approved, it will be merged with a [mergify script](https://github.com/hedyorg/hedy/blob/main/.mergify.yml). Please don't do anything (esp. don't enable auto merge), all will be handled automatically. Mergify will also tell you that when the PR is approved.
Expand Down
7 changes: 7 additions & 0 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,13 @@ def enrich_context_with_user_info():
return data


@app.context_processor
def add_generated_css_file():
return {
"generated_css_file": '/css/generated.full.css' if is_debug_mode() else '/css/generated.css'
}


@app.after_request
def set_security_headers(response):
security_headers = {
Expand Down
2 changes: 1 addition & 1 deletion build-tools/heroku/on-deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ cd $scriptdir
# for CSS classes in the HTML templates and JavaScript files).

echo '-----> Doing a Tailwind build'
tailwind/generate-css
tailwind/generate-prod-css

echo '-----> Compiling Babel translations'
(cd ../.. && pybabel compile -f -d translations)
Expand Down
14 changes: 6 additions & 8 deletions build-tools/heroku/tailwind/README.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
Tailwind for Hedy
=================

This directory contains the sources for Tailwind. Normally we'd include this in
our build process, but our build process is minimal and Python-based, and we
don't necessarily want to include NPM-based tooling into a Python build.
This directory contains two scripts:

That's why Tailwind CSS is periodically generated by hand, by running:

```
$ tailwind/generate-css
```
* `generate-prod-css`: gets run on deploy to generate minimal CSS which gets
served on the real site.
* `generate-development-css`: can be run by developers to regenerate the
full development CSS file. You should almost never need to do this, just
when you want to use Tailwind condition classes (like `hover:`, `group-hover:`, etc).
15 changes: 0 additions & 15 deletions build-tools/heroku/tailwind/generate-css

This file was deleted.

19 changes: 8 additions & 11 deletions build-tools/heroku/tailwind/generate-development-css
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
#!/bin/bash
# Generate a DEVELOPMENT copy of generated.css
#
# This is the version that should always be checked in. It is based on the FULL
# Tailwind config, so that developers can always program against all classes without
# having to worry about regenerating the CSS.
#
# At deployment time, we will use the stripping config build to make sure the generated
# CSS file ends up small.
set -eu
scriptdir=$(cd $(dirname $0) && pwd)
cd $scriptdir

echo "👉 Generating CSS with all features enabled. This file's goan' be big!" >&2

staticdir=../../../static
targetfile=generated.full.css

npx tailwindcss build -c $scriptdir/tailwind.full.js -i styles.css -o $staticdir/css/generated.css
npx minify $staticdir/css/generated.css > $staticdir/css/generated.css.min
mv $staticdir/css/generated.css{.min,}
if [[ "${1:-}" == "--watch" ]]; then
echo "👀 Running Tailwind compilation in watch mode. 👀"
npx tailwindcss build -c $scriptdir/tailwind.full.js -i styles.css -o $staticdir/css/$targetfile --watch
else
npx tailwindcss build -c $scriptdir/tailwind.full.js -i styles.css -o $staticdir/css/$targetfile
# Not minifying on purpose, to reduce merge conflicts if this file ever changes
fi
16 changes: 16 additions & 0 deletions build-tools/heroku/tailwind/generate-prod-css
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/bin/bash
set -eu
scriptdir=$(cd $(dirname $0) && pwd)
cd $scriptdir

staticdir=../../../static
targetfile=generated.css

if [[ "${1:-}" == "--watch" ]]; then
echo "👀 Running Tailwind compilation in watch mode. 👀"
npx tailwindcss build -c $scriptdir/tailwind.config.js -i styles.css -o $staticdir/css/$targetfile --watch
else
npx tailwindcss build -c $scriptdir/tailwind.config.js -i styles.css -o $staticdir/css/$targetfile
npx minify $staticdir/css/$targetfile > $staticdir/css/$targetfile.min
mv $staticdir/css/$targetfile{.min,}
fi
Loading

0 comments on commit b2ab78c

Please sign in to comment.