-
-
Notifications
You must be signed in to change notification settings - Fork 76
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 tailwind appendix #245
Conversation
✔️ Deploy Preview for ember-cli-guides ready! 🔨 Explore the source changes: 94a1d5c 🔍 Inspect the deploy log: https://app.netlify.com/sites/ember-cli-guides/deploys/61e055d6e99adb0007d53275 😎 Browse the preview: https://deploy-preview-245--ember-cli-guides.netlify.app |
Very nice! I have a few questions and suggestions. How much of these steps are specific to tailwind vs other libraries? It would be nice to have if we taught the overall strategy and then gave tailwind as the example. This is just an idea to take or leave. Change request: per our usual conventions for URLs for Ember resources, the URL should be a generic concept, and we maintain URLs forever. Libraries come and go in popularity, so this helps us to make sure that we can adapt library-specific content without killing URLs. The URL is separate from what is shown in the sidebar. We also usually try to name the sidebar topics in a generic way too, but in this case I think the word tailwind needs to be in there for discoverability. |
I kinda like this. The gist is that you can integrate any cli/watcher/builder utility with ember without ember-specific hooks. With the knowledge of what ember-cli watches and how assets can be accessed, anything is possible. I'm not sure how to best phrase that and use tailwind as an example though. :-\
makes sense! will change! |
Co-authored-by: Jen Weber <[email protected]>
Co-authored-by: Jen Weber <[email protected]>
Co-authored-by: Jen Weber <[email protected]>
Co-authored-by: Jen Weber <[email protected]>
My next steps as a reviewer are to check in with the rest of the learning team before we press "go." We do this for all new pages. We have our next meeting Thursday. I think you have addressed most of the things I could imagine as concerns. I left a couple more comments with some wording to help imply that Tailwind is not an "Ember" thing but part of the broad JS ecosystem. I also want to make sure we make it clear that most of the time, you install a dependency and it just works, but some things need a little more finesse. From my perspective, this can be shipped after those additions are included, or something similar to them. |
P.S. thank you so much for writing this up! A common theme we hear from users is that it's too hard to find info like this - it's lost in Discord chats. The more we can share, the better, IMO. |
@ember-learn/cli-core-team ready for rereview |
I believe the spacing issue is a known bug, agreed that it’s out of scope. |
If you want it to be different, I think you can add one more line of text below the bulleted list. I don’t think an extra return alone is enough. |
Thanks again for your work on this! And thanks for your patience. This slipped through my radar. |
Sorry for commenting on a merged PR, but I hadn't seen this one before. I noticed two things:
We should probably update this once Embroider is included by default? Thanks for doing this! This has helped me as well with Ember + Tailwind JIT! 👍 EDIT: Made #256. |
pnpm tells you to install them. They may be peer deps, but they aren't marked as optional. This totally could have changed since this PR was put up, but based what i'm seeing, tho warnings would still be present.
Aren't all assets minified by ember tho ? Minifying already minified code gets slow |
No, I don't think Ember does any optimization for assets in |
Oh i know, but i don't trust docs.
Excellent! Thanks! |
Resolves: #244