Integration Docs: Tailwind #3429
Conversation
|
github-actions
bot
added
the
pkg: integration
There was a problem hiding this comment.
Great work Dan! I like how this feels. I have a couple of thoughts/questions but I’m curious what others will say on these.
-
I love the videos! Does anyone know what happens to videos in READMEs when they’re rendered on npmjs.com? The README is the front page of the package there as well as on GitHub, so we need to think about that. They may vanish? And I think that could be OK if they are supplementary and we know that, but something to bear in mind.
-
I’m not 100% on collapsing the installation instructions in a
<details>element. It feels a bit better for the config section where the implication is that these are less commonly used parts of the API, but a user will want to install the package at some point so I feel we maybe shouldn’t hide those. Curious what others think though.Same question here too: how is
<details>rendered on npmjs.com? Couldn’t find any packages using it on a quick trawl.
|
|
||
| Changing `config.path` is well supported in Astro, but not recommended overall since it can cause problems with other Tailwind integrations, like the official Tailwind VSCode extension. | ||
| <details> | ||
| <summary>config.path</summary> |
There was a problem hiding this comment.
We can maintain the heading hierarchy/styling by setting a heading here I think. Does introduce some margin, so if that seems visually unappealing, maybe just <strong>?
| <summary>config.path</summary> | |
| <summary><h4>config.path</h4></summary> |
|
For the videos, my guess is it just gets rendered as a link that you can click on. For the install accordions, the idea was to prevent clutter on the page from two separate installation instructions, but I'm not particularly attached to them myself. |
| @@ -99,10 +126,31 @@ export default { | |||
| })], | |||
| } | |||
| ``` | |||
| </details> | |||
|
|
|||
| ## Examples | |||
There was a problem hiding this comment.
I love this examples section! (😉 )
I know this is just a Markdown README, but I'm wondering whether it might be worth trying to integrate some thumbnails here, especially for the themes we're recommending and the official Tailwind starter, just to jazz it up.
There was a problem hiding this comment.
Hmm maybe we can do that with a table? not sure
There was a problem hiding this comment.
I'm going to leave issues of display like this on the stack, as we develop our template. It will be easier to work on after it's up, and we start trying to make a nice display view. This is non-blocking, but it's on my radar.
|
|
||
| ## Contributing | ||
|
|
||
| This package is maintained by Astro's Core team. You're welcome to submit an issue or PR! |
There was a problem hiding this comment.
Depending on how much we want to encourage this, we could be linking to "new issue", "new PR" here. I know the reader is RIGHT HERE, so not terribly important. But I think a link demonstrates being open to contributions. Just depends if it fits the vibe... and if that's what we in fact want to encourage.
There was a problem hiding this comment.
This points to a broader problem of the New Issue button being on the entire Astro repo rather than this project... is there a way to link to creating a new issue with "Tailwind Integration:" already filled out in the header?
There was a problem hiding this comment.
Agreed. So I checked, and you can enable (and seems like withastro/astro has enabled) the You must choose a type of issue screen.
The problem is it looks like this, and only "bug report" actually files an issue in this repo. If you choose documentation, it's a link to the docs repo.
The most-proper but I think least-sensible option is to have another choice with bug report that is "correct integration documentation." But that is objectively terrible, especially given that to make issues with REGULAR documentation is already a choice and takes you somewhere different.
So, one imperfect option is to link directly to Docs issues where we will triage, and the workflow becomes to require an issue to Docs to tell us there's something wrong, where we can then guide them through the process if they want to actually submit a PR for it in the proper place. (The default option being that we make the fix ourselves, after the issue alerts us to any errors.)
|
Dan! I love this! Thanks for running with it. I've left some comments so far that I don't think should be blocking from porting this template to our other packages. Some minor fixes, some questions, some suggestions. I'll take another read through for this one page content in particular, but just wanted to add thoughts on the structure, fix obvious typos while I saw them, and give you a big thumbs up from me for the template in general! |
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
|
Thank you for your comments! I've made some changes in line with them - passing it on to you, @sarah11918! |
|
OK, my first task today is to get through this and make sure it's "mergeable." I'm going to deliberately not get hung up on things I can "fix (play around with) in post" so that it's up, and we can refine as we go if need be! |
|
@natemoo-re LGTM, too. We may eventually haggle over whether installation instructions should be behind details/summary, and may eventually try to incorporate some kind of screenshot/thumbnail into the Examples instead of just plain links. But, this is a "solid" effort by Dan, so let's get this merged, and move on to the next ones! |
This PR is a proposed template for integration documentation, presented here as an improvement to the Tailwind integration documentation.
Sarah and Chris know what I'm doing.