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

Add tailwindcss #29357

Merged
merged 19 commits into from
Feb 25, 2024
Merged

Add tailwindcss #29357

merged 19 commits into from
Feb 25, 2024

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Feb 24, 2024

This will get tailwindcss working on a basic level. It provides only the utility classes, e.g. no tailwind base which we don't need because we have our own CSS reset. Without the base, we also do not have their CSS variables so a small amount of features do not work and I removed the generated classes for them.

Note for future developers: This currently uses a tw- prefix, so we use it like tw-p-3.

Currently added CSS, all false-positives
.\!visible{

    visibility: visible !important
}

.visible{

    visibility: visible
}

.invisible{

    visibility: hidden
}

.collapse{

    visibility: collapse
}

.static{

    position: static
}

.\!fixed{

    position: fixed !important
}

.absolute{

    position: absolute
}

.relative{

    position: relative
}

.sticky{

    position: sticky
}

.left-10{

    left: 2.5rem
}

.isolate{

    isolation: isolate
}

.float-right{

    float: right
}

.float-left{

    float: left
}

.mr-2{

    margin-right: 0.5rem
}

.mr-3{

    margin-right: 0.75rem
}

.\!block{

    display: block !important
}

.block{

    display: block
}

.inline-block{

    display: inline-block
}

.inline{

    display: inline
}

.flex{

    display: flex
}

.inline-flex{

    display: inline-flex
}

.\!table{

    display: table !important
}

.inline-table{

    display: inline-table
}

.table-caption{

    display: table-caption
}

.table-cell{

    display: table-cell
}

.table-column{

    display: table-column
}

.table-column-group{

    display: table-column-group
}

.table-footer-group{

    display: table-footer-group
}

.table-header-group{

    display: table-header-group
}

.table-row-group{

    display: table-row-group
}

.table-row{

    display: table-row
}

.flow-root{

    display: flow-root
}

.inline-grid{

    display: inline-grid
}

.contents{

    display: contents
}

.list-item{

    display: list-item
}

.\!hidden{

    display: none !important
}

.hidden{

    display: none
}

.flex-shrink{

    flex-shrink: 1
}

.shrink{

    flex-shrink: 1
}

.flex-grow{

    flex-grow: 1
}

.grow{

    flex-grow: 1
}

.border-collapse{

    border-collapse: collapse
}

.select-all{

    user-select: all
}

.resize{

    resize: both
}

.flex-wrap{

    flex-wrap: wrap
}

.overflow-visible{

    overflow: visible
}

.rounded{

    border-radius: 0.25rem
}

.border{

    border-width: 1px
}

.text-justify{

    text-align: justify
}

.uppercase{

    text-transform: uppercase
}

.lowercase{

    text-transform: lowercase
}

.capitalize{

    text-transform: capitalize
}

.italic{

    font-style: italic
}

.text-red{

    color: var(--color-red)
}

.text-shadow{

    color: var(--color-shadow)
}

.underline{

    text-decoration-line: underline
}

.overline{

    text-decoration-line: overline
}

.line-through{

    text-decoration-line: line-through
}

.outline{

    outline-style: solid
}

.ease-in{

    transition-timing-function: cubic-bezier(0.4, 0, 1, 1)
}

.ease-in-out{

    transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1)
}

.ease-out{

    transition-timing-function: cubic-bezier(0, 0, 0.2, 1)
}

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 24, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 24, 2024
@silverwind silverwind added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Feb 24, 2024
@silverwind silverwind removed the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Feb 24, 2024
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Feb 24, 2024

@yardenshoham
Copy link
Member

I thought we had gt- classes instead of tailwind

@silverwind
Copy link
Member Author

silverwind commented Feb 24, 2024

I don't want to introduce it with a prefix, it would just far too cumbersome to use. There are provenly no issues as seen in its CSS output into index.css and we can always extend its blocklist.

Migration and removal of gt- classes can happen anytime after this, I prefer separate PRs for those.

Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Considering there were already a lot of bugs caused by CSS class name conflicting, I believe using proper prefix for a new framework is a must.

This is a strong requirement from my side. Unless someone could prove that there won't be any conflict and the newly introduced names could be easily searched & replaced & maintained.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 24, 2024
@silverwind
Copy link
Member Author

silverwind commented Feb 24, 2024

Check the Currently added CSS in OP which at least proves no conflicts with first party HTML. I grepped all of them. The only case where conflicts could arise is with third-party HTML.

In fact, setting the gt- prefix now would introduce potential problems with the margin/padding classes because p-3 is 1rem in our CSS and 0.75rem in tailwind, those need to be migrated later.

@wxiaoguang

This comment was marked as outdated.

@silverwind
Copy link
Member Author

We can arrange the CSS so that any conflicting classes can be overridden easily in our helpers.css. Tailwind currently does not generate hidden likely because it can't find it anywhere in templates or js.

@wxiaoguang
Copy link
Contributor

We can arrange the CSS so that any conflicting classes can be overridden easily in our helpers.css. Tailwind currently does not generate hidden likely because it can't find it anywhere in templates or js.

Keeping adding names into "blacklist" is also problematic. When Gitea says that "it supports tailwind", then I try to use some "blacklisted" CSS names, then nothing happens or strange thing happens: it is not friendly to developers, just wastes time again.

Adding a proper prefix like tw- resolves every problem.

@silverwind
Copy link
Member Author

Adding a proper prefix like tw- resolves every problem.

Yes, but it offloads the problem to each developer having to type more. Well I guess at least we are already used to that with the gt- prefix.

@silverwind
Copy link
Member Author

Here's your damn prefix. I strongly dislike it as I'm sure it would have worked without but if you prefer to place extra burden on the developers, so be it.

We should aim to remove both the tw- and gt- prefixes eventually and aim to only use tailwind.

@wxiaoguang
Copy link
Contributor

but if you prefer to place extra burden on the developers, so be it.

I wouldn't call it a burden. At least, it's better than undetectable regressions and strange quirks (lacking some CSS classes).

We should aim to remove both the tw- and gt- prefixes eventually and aim to only use tailwind.

I could agree, I can see that gt- could be replaced by tw- soon, but I think Fomantic UI's CSS is still a blocker for this object.

@wxiaoguang wxiaoguang dismissed their stale review February 24, 2024 11:45

dismiss review

@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Feb 24, 2024
templates/repo/header.tmpl Outdated Show resolved Hide resolved
tailwind.config.js Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 24, 2024
@techknowlogick
Copy link
Member

Yeah, a quick +1 of what @denyskon said. Use the prefix now, and then remove it later once fomantic has been refactored out.

And a general 🎉 for this PR, tyvm for your work on this.

Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Tested with various tmpl syntax, for most cases it works well.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 25, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 25, 2024
@silverwind silverwind merged commit f4b9257 into go-gitea:main Feb 25, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Feb 25, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 25, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 26, 2024
* giteaofficial/main: (45 commits)
  Include resource state events in Gitlab downloads (go-gitea#29382)
  Add API to get PR by base/head (go-gitea#29242)
  [skip ci] Updated translations via Crowdin
  Improve Documentation for Restoration from backup (go-gitea#29321)
  Refactor "user/active" related logic (go-gitea#29390)
  Remove jQuery AJAX from the archive download links (go-gitea#29380)
  Add tailwindcss (go-gitea#29357)
  Add missing space (go-gitea#29393)
  Integrate alpine `noarch` packages into other architectures index (go-gitea#29137)
  enforce maxlength in frontend (go-gitea#29389)
  Remove incorrect and unnecessary Escape from templates (go-gitea#29394)
  Make actions animation rotate counterclockwisely (go-gitea#29378)
  Use `crypto/sha256` (go-gitea#29386)
  Add `io.Closer` guidelines (go-gitea#29387)
  Remove jQuery AJAX from the notice selection deletion button (go-gitea#29381)
  Refactor Safe modifier (go-gitea#29392)
  Add attachment support for code review comments (go-gitea#29220)
  Refactor modules/git global variables (go-gitea#29376)
  Remove jQuery from the code diff expansion buttons (go-gitea#29385)
  Remove jQuery AJAX from the markdown editor preview (go-gitea#29384)
  ...
@silverwind silverwind deleted the tailwind branch February 26, 2024 10:07
silverwind pushed a commit that referenced this pull request Feb 27, 2024
Follow #29357 
- Replace `gt-w-*` -> `tw-w-*` and remove `gt-w-*`
- Replace `gt-h-*` -> `tw-h-*` and remove `gt-h-*`
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/internal size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants