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

feat(windicss): add windicss ui setup #5194

Merged
merged 19 commits into from
Apr 20, 2022

Conversation

Olyno
Copy link
Contributor

@Olyno Olyno commented Apr 15, 2022

Description

Hi 👋🏻
Here is a pull request to implement the WindiCSS UI library. It is a great alternative to Tailwind, with fewer configurations, and more utilities.

Currently, this pull request should be tested. Due to a conflict with local packages from the Framework project, I wasn't able to test what I've done so far.

@netlify
Copy link

netlify bot commented Apr 15, 2022

Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit d8eba11
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/625fa8c83acc6b000843b003

@jtoar jtoar added topic/cli release:feature This PR introduces a new feature labels Apr 16, 2022
Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

Hey @Olyno I've used windicss once and it was a great experience. I tested this out locally and left a few comments. You can also lint fix to get some of the GitHub warnings to go away.

Due to a conflict with local packages from the Framework project, I wasn't able to test what I've done so far

Mind expanding on this? I was able to get things hooked locally up using yarn rwfw. And as long as you run the command with --no-isntall, node_modules won't be reset (and then you can install after the setup command). Did you have a hard time with the contributing docs? You can also try Gitpod, it's often just an easier experience.

packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
@jtoar jtoar self-assigned this Apr 16, 2022
@Olyno
Copy link
Contributor Author

Olyno commented Apr 16, 2022

Mind expanding on this? I was able to get things hooked locally up using yarn rwfw. And as long as you run the command with --no-isntall, node_modules won't be reset (and then you can install after the setup command). Did you have a hard time with the contributing docs? You can also try Gitpod, it's often just an easier experience.

When I tried it locally, I encountered some problems. For example, when running the command "yarn rwfw", I got an error that it was not possible to find the workspace "web":

I also got an error at one point with the dependencies:

I honestly don't know how to explain these issues. I didn't try Gitpod, but it can be a good option if it works better. I would have highlighted it in the documentation in these cases.

There's a thread on the Discord with probably more information if you're curious to find out what's wrong).

Also, I think that the "Test the CLI" part should be before the "Test the Framework" part because it seems that we don't need sync, which would allow us to go straight to the concrete.

@thedavidprice
Copy link
Contributor

@jtoar re: local dev topic — we have two next steps to address now that we're beyond 1.0:

  1. given that the yarn dev command is hit or miss, we need to update documentation to clarify (i.e. experimental/incomplete) or remove entirely
  2. we still need to resolve yarn rwfw project:copy issues (due to yarn 3), which may also be affecting rwfw project:sync. Behavior so far has been that project:copy works once but on subsequent uses it break node_modules

Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

@Olyno here's another round of reviews. I think it's good to go after all these are addressed.

packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/ui/libraries/windicss.js Outdated Show resolved Hide resolved
Olyno and others added 14 commits April 17, 2022 16:09
Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com>
Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com>
Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com>
Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com>
Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com>
Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com>
@jtoar jtoar marked this pull request as ready for review April 20, 2022 06:26
Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

Thanks @Olyno, this looks great. Let's get this one merged

@jtoar jtoar enabled auto-merge (squash) April 20, 2022 06:40
@jtoar jtoar merged commit d4ec2f9 into redwoodjs:main Apr 20, 2022
@jtoar jtoar added this to the next-release milestone Apr 20, 2022
@Olyno Olyno deleted the feature/setup-ui-windicss branch April 20, 2022 08:11
@thedavidprice
Copy link
Contributor

Thanks for the work here @Olyno And most of all for taking me up on the invitation — well done 🚀

@jtoar can you make sure Docs for this are updated accordingly?

@Tobbe is there anything that needs to be connected between this PR and the work in #4900

?

@Tobbe
Copy link
Member

Tobbe commented Apr 20, 2022

@Tobbe is there anything that needs to be connected between this PR and the work in #4900

No, it's all good. They're both touching App.{js,tsx} but 1) you most likely won't have both at the same time, and 2) with all the advanced merge stuff going on in #4900 I'm sure it can handle it anyway 🙂

@thedavidprice thedavidprice modified the milestones: next-release, v1.2.0 Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature topic/cli
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants