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

Tailwind configuration #32

Merged
merged 15 commits into from
Nov 8, 2022
Merged

Conversation

janbaykara
Copy link
Collaborator

@janbaykara janbaykara commented Nov 7, 2022

This PR adds the foundational design tokens from the Figma, based on Flowbite.

  • Type scale
  • Font families
  • Spacing scale
  • Colours
  • Default text
  • Rich text rendering

Motivation and Context

Context here is that, because we're using Flowbite as the underlying design system, it should be easy to check Figma for the correct classnames to apply to precisely produce the desired styling in HTML and CSS.

There are three CSS files that we should add stuff to, if required:

  • frontend/scss/base.scss: this is for basic overriding styles that apply globally. Be very careful about this. Most always you'll want to define components or utilities and apply these in a scoped context in HTML.
  • frontend/scss/components.scss: this is for composing utilities together into semantically named classes. For example, I can imagine us pretty soon implementing standard div.label and a.link component classes for use around the site in different places, although not all div and a elements want to be automatically styled that way.
  • frontend/scss/utilities.scss: this is the place to add more tailwind-style class utilities here, named according to their visual effect (e.g. text-blue)

How Can It Be Tested?

As you build out UI, if you find that these classes aren't producing the expected visual output compared to Figma, we need to update the Tailwind file.

How Will This Be Deployed?

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I've checked the spec (e.g. Figma file) and documented any divergences.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I've updated the documentation accordingly.
  • Replace unused checkboxes with bullet points.

@janbaykara janbaykara requested a review from conatus November 7, 2022 17:23
@linear
Copy link

linear bot commented Nov 7, 2022

HOT-219

@conatus conatus changed the title Tailwind config Tailwind configuration Nov 7, 2022
Copy link
Collaborator

@conatus conatus left a comment

Choose a reason for hiding this comment

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

Lovely stuff. Can we agree on a direction on consistent HTML quote usage please and then proceed?

app/templates/app/article_page.html Outdated Show resolved Hide resolved
app/templates/app/directory.html Outdated Show resolved Hide resolved
@conatus
Copy link
Collaborator

conatus commented Nov 7, 2022

The lint step of the CI job is failing. If you can see a quick fix, then we should fix. If it is more tricky, feel with should take an issue and proceed, a little later.

@janbaykara
Copy link
Collaborator Author

Agreed that the HTML autoformatting would be nice. Unfortunately prettier breaks django template language syntax, will have a quick google for an alternative but otherwise will park that request in the backlog.

@janbaykara
Copy link
Collaborator Author

Cool, now we have a pretty good Django HTML linter and code formatter! It will auto-reindent HTML which is nice, and will alert us to bad HTML practice like mixed quotes and unclosed tags and things like that — we'll have to fix them but they're pretty easy to fix. This has been added to:

  • pre-commit, which will reindent HTML and output lint errors
  • ci, which will output lint errors

Example linting error:

app/templates/base.html

H008 39:8 Attributes should be double quoted. <div class='flex fle
H017 40:12 Tag should be self closing. <br>

@janbaykara
Copy link
Collaborator Author

janbaykara commented Nov 8, 2022

Oh god, not this setuptools nightmare again... I previously saw this in https://github.com/commonknowledge/leftbookclub

python-poetry/poetry#4242

@janbaykara janbaykara requested a review from conatus November 8, 2022 16:59
@janbaykara
Copy link
Collaborator Author

Wehey! @conatus I managed to solve all sorts of CI issues alongside Django HTML formatting and linting.

Copy link
Collaborator

@conatus conatus left a comment

Choose a reason for hiding this comment

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

Good work. Feels like a stronger version of the CI setup which will ensure a higher code quality immediately, so worthwhile in the round.

@janbaykara janbaykara merged commit a0989d0 into main Nov 8, 2022
@janbaykara janbaykara mentioned this pull request Nov 8, 2022
7 tasks
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.

2 participants