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

Attempt to fix hiding of attribute described in markdown. #139

Merged
merged 11 commits into from
Nov 18, 2024

Conversation

brettle
Copy link
Contributor

@brettle brettle commented Nov 17, 2024

Summary of Changes

  1. Attempted to fix the fact that an attribute described in MD was not being rendered. It renders correctly on GitHub but not on the docs site, presumably due to different MD flavors.

Copy link

netlify bot commented Nov 17, 2024

Deploy Preview for super-tapioca-5987ce ready!

Name Link
🔨 Latest commit faba07d
🔍 Latest deploy log https://app.netlify.com/sites/super-tapioca-5987ce/deploys/673b9ac3072f5800085c3790
😎 Deploy Preview https://deploy-preview-139--super-tapioca-5987ce.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@brettle
Copy link
Contributor Author

brettle commented Nov 17, 2024

Well the preview indicates that using entities was definitely not the correct fix here. Not sure what is.

@brettle brettle marked this pull request as draft November 17, 2024 18:43
@brettle brettle marked this pull request as ready for review November 17, 2024 19:46
@brettle
Copy link
Contributor Author

brettle commented Nov 17, 2024

Removing the quotes was the only fix I could find. Updating unified to a more recent version might also fix the problem but I'm not in a position to test that easily.

@thescientist13
Copy link
Member

thescientist13 commented Nov 17, 2024

Huh, that's interesting behavior. Not sure if you tried this, but would using an HTML entity work here?

for these components by adding the `data-gwd-opt=&quote;static&quote;` attribute 

Updating unified to a more recent version might also fix the problem but I'm not in a position to test that easily.

Yeah, they are definitely a bit outdated. I think we would like to move markdown to its own standalone plugin (in the event someone doesn't want to use unified, and so that would be a good time to give these all full upgrade - ProjectEvergreen/greenwood#1247)


edit: sorry, just your comment (#139 (comment)). Let me try and take look

@thescientist13 thescientist13 added bug Something isn't working docs Greenwood specific content like docs and guides labels Nov 17, 2024
@thescientist13
Copy link
Member

thescientist13 commented Nov 18, 2024

Hmm, interestingly it seems to display fine when just running the dev server 🤔
Screenshot 2024-11-18 at 12 16 58 PM

What's even more interesting is that there's another similar case here but it renders fine?
https://greenwoodjs.dev/docs/resources/scripts/#modules-esm
Screenshot 2024-11-18 at 12 26 56 PM

@thescientist13
Copy link
Member

thescientist13 commented Nov 18, 2024

So, with some more testing, if I use single quotes, it seems fine (might be the short term solution for now)
Screenshot 2024-11-18 at 12 31 37 PM

Or removing the hyphens
Screenshot 2024-11-18 at 12 30 17 PM

&quote works but with the literal text
Screenshot 2024-11-18 at 12 33 31 PM


Let me test now with bumping all the dependencies and or mixing / matching, to see if it's coming from somewhere down in the stack.

@brettle
Copy link
Contributor Author

brettle commented Nov 18, 2024

Just fyi, I think the entity is " not &quote; (no trailinge).

@thescientist13
Copy link
Member

thescientist13 commented Nov 18, 2024

Ah, thanks @brettle . That one still seems to lead to the same outcome.

I tried upgrading all the related markdown deps and even commented out our custom markdown plugins and it still disappears. Even if I use a code fence block instead of inline one 😕

```html
data-gwd-opt="static"
```

Screenshot 2024-11-18 at 1 14 53 PM

So I think this is something a little more involved, maybe on the Greenwood side.


I think let's go with single quotes for now since that seems to work at least, and let me track this on the Greenwood side.

@@ -37,7 +37,7 @@ Now if we look in the HTML output for any of our pages, we will see pre-rendered
</app-footer>
```

We can go one step further and instruct Greenwood to strip out the `<script>` tags for these components by adding the `data-gwd-opt="static"` attribute to them, since we have no need for any interactivity on these components.
We can go one step further and instruct Greenwood to strip out the `<script>` tags for these components by adding the `data-gwd-opt=static` attribute to them, since we have no need for any interactivity on these components.
Copy link
Member

Choose a reason for hiding this comment

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

it seems like single quotes works at least, so we can just roll with that for now

Copy link
Member

@thescientist13 thescientist13 left a comment

Choose a reason for hiding this comment

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

Thank you so much for going down the rabbit hole on this one. 🐰

Opened an issue to track this on the Greenwood side and hopefully figure out what is going on here 😵

@thescientist13 thescientist13 merged commit 0b0d0cd into ProjectEvergreen:main Nov 18, 2024
5 checks passed
@brettle brettle deleted the patch-6 branch November 18, 2024 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docs Greenwood specific content like docs and guides
Projects
Development

Successfully merging this pull request may close these issues.

2 participants