-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 a warning about dynamic tags and reword another in Template Directives #6477
Comments
Thank you for this thoughtful analysis, @ArmandPhilippot and thank you for your patience as we work through issues ! I can tell you that I am also very appreciative of your concern about adding multiple caution blocks together! You're right, that is a pattern we really try to avoid, because usually a smell that content needs reorganizing. I will call attention to this with some of our devs who can help work through what docs content might need to change, and then depending on what we decide on, we'd be happy to have you make a PR to docs to make the section more helpful! |
No problem, I understand that it is not a priority. Especially since I thought I saw a proposal to replace |
So, I'm getting back to this now (after some time, I know!) and since we still do have Your text addition proposals look great, but yes, we would prefer not to have a bunch of notes and cautions and stuff around. I'm proposing:
Do you think these proposals would satisfy the issues you've identified? If so, I'd be happy to receive a PR if you'd like to make one yourself! (If not, let me know and I can make the appropriate changes.) |
Thank you for the feedback and no worries about the elapsed time. I'm fine with your proposals so I opened #8585! |
📚 Subject area/topic
Templates directives
📋 Page(s) affected (or suggested, for new content)
https://docs.astro.build/en/reference/directives-reference/
📋 Description of content that is out-of-date or incorrect
I noted two improvements more or less related that can be made on the "Template Directives" page. I could open a pull request but:
define:vars
sectionThe issue
Using
define:vars
on a<style />
tag in a component which depends on dynamic tags does not work. An issue was reported on the Astro repository but has been closed. It seems a fix is not planned. So I think it would be worth mentioning it in the documentation.My concerns
There is already a warning about
<script />
tags:So I'm not sure what is the best approach to differentiate the two warnings:
<script />
and a note block for<style />
(I saw an example of two different blocks next to each other in the documentation),<style />
tag case as less important and to put it in a new paragraph between the code block and the warning block (there is a similar mention on the "Client directives" section: "Hydration directives are not supported when using dynamic tags".Proposal
Regardless of the chosen format, we could add the following sentences:
is:inline
sectionIssue
There is a warning block about the directive being applied automatically for any attribute other than
src
for both<script />
and<style />
tags:But in the next section (
define:vars
), the warning only mentions the<script />
tag:There was a previous issue (#3165) about
is:inline
behavior withdefine:vars
. It seems the statement inis:inline
section is wrong: usingdefine:vars
on<style />
tag does not automatically apply theis:inline
directive.So I think the warning block in
is:inline
section should be reworded to reflect the current behavior.Proposal
Perhaps I'm wrong but the
lang
attribute (ie.<style lang="scss">
) does not apply theis:inline
directive. Maybe it should be added to the list to prevent confusion.🖥️ Reproduction in StackBlitz (if reporting incorrect content or code samples)
About the
define:vars
bug, here is the StackBlitz from the original issue (not mine): https://stackblitz.com/edit/astro-minimal-example-custom-tags?file=README.mdThe text was updated successfully, but these errors were encountered: