Skip to content

Transition to Tailwind CSS#5049

Merged
ekowidianto merged 4 commits intomasterfrom
phillmont/lets-go-tailwindcss
Sep 7, 2022
Merged

Transition to Tailwind CSS#5049
ekowidianto merged 4 commits intomasterfrom
phillmont/lets-go-tailwindcss

Conversation

@pchunky
Copy link
Copy Markdown
Contributor

@pchunky pchunky commented Sep 2, 2022

Note: This PR has phillmont/new-edit-assessment (#5034) as a base branch.


Transition to Tailwind CSS Tailwind CSS

Tailwind CSS is a utility-first CSS framework for rapidly building custom user interfaces. It offers a different paradigm of traditionally writing 'semantic' CSS, allowing for definitive stylesheet consistency, ease of configuration, and better developer experience. Read how Adam Wathan, the creator of Tailwind CSS, convinced the world that utility-first CSS is a thing, and brought Tailwind CSS to one of the best and most popular CSS frameworks.

TL;DR

  1. Moving forward, we are using Tailwind's CSS utilities for any styling. Ideally, MUI is only used to provide components.
  2. Generally, there mustn't be sx or style props/attributes being used.
  3. Inline styles, raw CSS, Sass stylesheets, styled-components, or Emotion are henceforth illegal.
  4. As much as possible, refrain from using arbitrary values, even if Tailwind supports it.
    If you really have to, refrain from using absolute values. Use rem, not em.
  5. If you are deep styling MUI components, i.e., modifying very specific parts of MUI components, use their official APIs, e.g., componentProps, InputLabelProps , InputComponentProps, etc., to provide Tailwind's classNames. This is MUI's official recommendation, anyway.
    Unless really necessary, do NOT directly target MUI's class names, e.g., .MuiDialog-root, because ideally, these are MUI's internal implementation details that might change in their next versions.
  6. See an example of the New/Edit Assessment form's styles rewritten in Tailwind here.

Things to note when developing with Tailwind for now

The Tailwind's stock utilities are provided in index.css. This is also where you will put your custom utilities, if really needed. It must be loaded by the root React instance, ideally the one who calls ReactDOM.render. Since our SPA is upcoming, index.css is loaded by client/app/index.js.

The default React #root element is self-assigned in course.html.slim. If other modules need Tailwind utilities, append the #root ID to the parent HTML element.

If your Tailwind utilities do not work on any Portal elements, set their container to the rootElement (like Dialog, Popover, and Popper here) so they are mounted there instead of document.body.

Some metric, if you care

As of this PR, with only some components' styles converted to Tailwind's utilities (see Files changed), their bundle sizes reduced by 3-5%, analyzed with Bateman's Webpack Visualizer. As more styles are moved to Tailwind, our bundle sizes should reduce, but only by a tad bit.

Notably, performance or bundle size is not the point of this transition. It is all about the developers' experience and writing stylesheets that are scalable and do not grow (sharply) as our React code grows.

@pchunky pchunky added Enhancement UI JavaScript Pull requests that update JavaScript code labels Sep 2, 2022
@pchunky pchunky self-assigned this Sep 2, 2022
@pchunky pchunky marked this pull request as draft September 2, 2022 10:35
@ekowidianto ekowidianto changed the base branch from master to phillmont/new-edit-assessment September 2, 2022 10:38
@ekowidianto ekowidianto changed the base branch from phillmont/new-edit-assessment to master September 2, 2022 10:38
@pchunky pchunky force-pushed the phillmont/lets-go-tailwindcss branch from 17e989c to d4cdadd Compare September 2, 2022 12:39
@pchunky pchunky changed the base branch from master to phillmont/new-edit-assessment September 2, 2022 12:39
@pchunky pchunky force-pushed the phillmont/new-edit-assessment branch 2 times, most recently from b5cf6f1 to 712f6fe Compare September 5, 2022 03:22
@pchunky pchunky force-pushed the phillmont/lets-go-tailwindcss branch 4 times, most recently from 2c97d4b to 5b8b718 Compare September 5, 2022 09:48
@pchunky pchunky marked this pull request as ready for review September 5, 2022 09:49
@pchunky pchunky force-pushed the phillmont/lets-go-tailwindcss branch from 5b8b718 to d377138 Compare September 5, 2022 10:30
@pchunky pchunky requested a review from ekowidianto September 5, 2022 10:30
@ekowidianto ekowidianto force-pushed the phillmont/new-edit-assessment branch from 712f6fe to 4198e59 Compare September 6, 2022 03:12
@ekowidianto
Copy link
Copy Markdown
Member

@purfectliterature Please rebase thanks!

Base automatically changed from phillmont/new-edit-assessment to master September 6, 2022 03:26
@pchunky pchunky force-pushed the phillmont/lets-go-tailwindcss branch from d377138 to e2d419a Compare September 6, 2022 04:52
@pchunky
Copy link
Copy Markdown
Contributor Author

pchunky commented Sep 6, 2022

I found that leaderboard_viewing_spec tests for clicking the #achievement-tab button in LeaderboardIndex that only appears in screen sizes smaller than the lg breakpoint. I suppose because I matched our MUI breakpoints to Tailwind's breakpoints, lg reduced from 1200px (MUI default) to 1024px (Tailwind default), then because probably the test suite's chromedriver has screen size >1024px, the #achievement-tab button disappears and thus the tests failed.

I committed a fix 732883d that allows leaderboard_viewing_spec to test properly regardless of screen size. We should probably take note to write E2E tests that are media-agnostic.

Copy link
Copy Markdown
Member

@ekowidianto ekowidianto left a comment

Choose a reason for hiding this comment

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

Great job @purfectliterature!! Thanks for introducing this framework. I like it just because TLDR - ‘Tailwind is a constrained inline style that allows responsive design and hover, focus and other states styling’.

A few things to note:

  1. Do we want to consider using this plugin to optimize prod build? Probably this wont help much now but I guess this is good for future development. https://tailwindcss.com/docs/optimizing-for-production
  2. Could you help adding some docs on the usage for react styling in ‘Readme.md’ in the ‘client’ folder? We will need to start populating this file with more convention, guideline and usage info for our project (eg redux, intl, router, testing, etc).
  3. Please add linter/eslint.
  4. In the future, should we use tailwind theme or preset to centralize styling, theme ,color ,etc control and how/if do we reconcile with MUI?

Comment thread client/tailwind.config.js
Comment thread client/app/lib/components/IconRadio.tsx Outdated
Comment thread client/app/lib/components/IconRadio.tsx
Comment thread client/app/lib/components/TextField.tsx
Comment thread client/app/lib/components/ProviderWrapper.jsx
@pchunky
Copy link
Copy Markdown
Contributor Author

pchunky commented Sep 7, 2022

  1. Do we want to consider using this plugin to optimize prod build? Probably this wont help much now but I guess this is good for future development. https://tailwindcss.com/docs/optimizing-for-production

I have since set up cssnano as of 8780a4c.

  1. Could you help adding some docs on the usage for react styling in ‘Readme.md’ in the ‘client’ folder? We will need to start populating this file with more convention, guideline and usage info for our project (eg redux, intl, router, testing, etc).

I will add the documentations after I spend some time with them when I migrate the Course Settings page. I have added an issue #5067 for me to track.

  1. Please add linter/eslint.

Tailwind has linting built into their Tailwind CSS IntelliSense extension for Visual Studio Code. This extension includes both linting and IntelliSense for their utilities, as well as the raw CSS outputs to see the actual rem values or actual colours.

  1. In the future, should we use tailwind theme or preset to centralize styling, theme ,color ,etc control and how/if do we reconcile with MUI?

Yes, we should centralise everything in Tailwind, more specifically tailwind.config.js. This will make it breezy when it comes to custom theming in the future. This includes breakpoints (done), colours, text systems, spacing, border styles, transitions, animations, and/or others that I cannot think on top of my head right now.

Text systems and colours can be a bit iffy, because we'd probably still want to be able to access these via MUI's props, e.g., <Typography variant="body1" ...> or <Icon color="text.secondary" ...>. We probably will need to formalise our colour palette to tailwind.config.js and retrieve the colours to MUI Theme. Otherwise, we can only do <Icon className="text-gray-200" ...> for colour, or (a) <Typography className="text-sm" ...> or (b) create our own @layer utilities { .text-body-1 { ... } } in index.css and do <Typography className="text-body-1" ...>, for text systems.

I did not set this up in this PR because (a) it could affect font/colour styles in many, many pages, and (b) our transition to Tailwind is still under probation anyway. Let me fully test-drive Tailwind in my Course Settings migration first, then once everything is okay and there is no gotchas, I will set up the remaining bits and pieces of MUI x Tailwind; we can merge this PR in first.

@pchunky pchunky force-pushed the phillmont/lets-go-tailwindcss branch from 18681ec to 8780a4c Compare September 7, 2022 05:09
@ekowidianto
Copy link
Copy Markdown
Member

  1. Do we want to consider using this plugin to optimize prod build? Probably this wont help much now but I guess this is good for future development. https://tailwindcss.com/docs/optimizing-for-production

I have since set up cssnano as of 8780a4c.

  1. Could you help adding some docs on the usage for react styling in ‘Readme.md’ in the ‘client’ folder? We will need to start populating this file with more convention, guideline and usage info for our project (eg redux, intl, router, testing, etc).

I will add the documentations after I spend some time with them when I migrate the Course Settings page. I have added an issue #5067 for me to track.

  1. Please add linter/eslint.

Tailwind has linting built into their Tailwind CSS IntelliSense extension for Visual Studio Code. This extension includes both linting and IntelliSense for their utilities, as well as the raw CSS outputs to see the actual rem values or actual colours.

  1. In the future, should we use tailwind theme or preset to centralize styling, theme ,color ,etc control and how/if do we reconcile with MUI?

Yes, we should centralise everything in Tailwind, more specifically tailwind.config.js. This will make it breezy when it comes to custom theming in the future. This includes breakpoints (done), colours, text systems, spacing, border styles, transitions, animations, and/or others that I cannot think on top of my head right now.

Text systems and colours can be a bit iffy, because we'd probably still want to be able to access these via MUI's props, e.g., <Typography variant="body1" ...> or <Icon color="text.secondary" ...>. We probably will need to formalise our colour palette to tailwind.config.js and retrieve the colours to MUI Theme. Otherwise, we can only do <Icon className="text-gray-200" ...> for colour, or (a) <Typography className="text-sm" ...> or (b) create our own @layer utilities { .text-body-1 { ... } } in index.css and do <Typography className="text-body-1" ...>, for text systems.

I did not set this up in this PR because (a) it could affect font/colour styles in many, many pages, and (b) our transition to Tailwind is still under probation anyway. Let me fully test-drive Tailwind in my Course Settings migration first, then once everything is okay and there is no gotchas, I will set up the remaining bits and pieces of MUI x Tailwind; we can merge this PR in first.

Great thanks!

@ekowidianto ekowidianto enabled auto-merge (rebase) September 7, 2022 05:19
@ekowidianto ekowidianto merged commit 17e4000 into master Sep 7, 2022
@ekowidianto ekowidianto deleted the phillmont/lets-go-tailwindcss branch September 7, 2022 05:27
@pchunky pchunky mentioned this pull request Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement JavaScript Pull requests that update JavaScript code UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants