Skip to content

Documentation enhancement: support markdown alerts#2054

Merged
yuri-kiss merged 28 commits intoTurboWarp:masterfrom
Brackets-Coder:docs-enhancement
Mar 31, 2025
Merged

Documentation enhancement: support markdown alerts#2054
yuri-kiss merged 28 commits intoTurboWarp:masterfrom
Brackets-Coder:docs-enhancement

Conversation

@Brackets-Coder
Copy link
Contributor

@Brackets-Coder Brackets-Coder commented Mar 27, 2025

#2050

This is a simple pull request that adds support for alerts in extension documentation markdown files:

Note

Useful information that users should know, even when skimming content.

Tip

Helpful advice for doing things better or more easily.

Important

Key information users need to know to achieve their goal.

Warning

Urgent info that needs immediate user attention to avoid problems.

Caution

Advises about risks or negative outcomes of certain actions.

Some of the CSS is messy and the rule was poorly designed and I kind of rushed this

@github-actions github-actions bot added the pr: other Pull requests that neither add new extensions or change existing ones label Mar 27, 2025
improves consistency
Copy link
Member

@CubesterYT CubesterYT left a comment

Choose a reason for hiding this comment

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

Haven't tested yet, but here are some changes I think could be made

@CubesterYT CubesterYT linked an issue Mar 27, 2025 that may be closed by this pull request
@CubesterYT
Copy link
Member

I'll test this now.

@CubesterYT
Copy link
Member

image

Seems to work great! (I'm surprised my suggested changes didn't break anything, this was my first time handling that area.)

@CubesterYT
Copy link
Member

Now the next step going forward, is removing the tests in Simple3D, and actually go through all documentations and utilizing these new features, and then this PR should be ready

@Brackets-Coder
Copy link
Contributor Author

Now the next step going forward, is removing the tests in Simple3D, and actually go through all documentations and utilizing these new features, and then this PR should be ready

Must every extension utilize these? I think they would be more efficient in larger extensions like Simple3D and AR but I don't think things like Bitwise or local storage should have them

@CubesterYT
Copy link
Member

Must every extension utilize these? I think they would be more efficient in larger extensions like Simple3D and AR but I don't think things like Bitwise or local storage should have them

I meant looking through documentations that actually could utilize these. I know some docs actually have things like notes/important/warnings, etc., and we could update them with these.

@Brackets-Coder
Copy link
Contributor Author

Seems to work great! (I'm surprised my suggested changes didn't break anything, this was my first time handling that area.)

I'm not surprised, CSS isn't too hard to learn

@Brackets-Coder
Copy link
Contributor Author

I've added a warning to the "precision mode" performance note in Box2D. I figure the best way to go about including these in documentation is to just go through all the extensions in order one by one

@Brackets-Coder

This comment was marked as resolved.

@Brackets-Coder

This comment was marked as outdated.

This one is tricky, because the extension's colors are purple and the "important" alert doesn't stand out immediately. A "tip" would be better color-wise but doesn't really fit the context.
Copy link

@yuri-kiss yuri-kiss left a comment

Choose a reason for hiding this comment

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

Other than this it looks good to me


const match =
/^\s*>\s*\[!(NOTE|TIP|IMPORTANT|WARNING|CAUTION)]\s*(.*)/.exec(marker);
if (!match) return false;

This comment was marked as abuse.

This comment was marked as abuse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, the build was failing and I wanted it to fail gracefully, but I'll add the throw. Thanks for clarifying

@Brackets-Coder
Copy link
Contributor Author

Not sure if I fully understood what you were asking for, but I think this should do it?

@Brackets-Coder
Copy link
Contributor Author

No, I just broke it.

@Brackets-Coder
Copy link
Contributor Author

Screenshot 2025-03-28 at 12 50 56 PM

I think this is what you meant

@yuri-kiss

This comment was marked as abuse.

@Brackets-Coder
Copy link
Contributor Author

Brackets-Coder commented Mar 28, 2025

@yuri-kiss doing your suggested change always returns an error of match null (even when using a valid alert type) and doing it for the icon accepts incorrect syntax.

@yuri-kiss

This comment was marked as abuse.

@Brackets-Coder
Copy link
Contributor Author

I understood what to do, but I was a bit busy doing other things but adding the TypeError makes it fail every time even if there isn't an error...
Screenshot 2025-03-29 at 3 33 52 PM
Screenshot 2025-03-29 at 3 35 58 PM

The second image is from Local Storage's documentation, which has NO alerts but it still returns an error...

@Brackets-Coder
Copy link
Contributor Author

@yuri-kiss it's not working and fails on every documentation page immediately, even if there is no alert. Also fails for alerts done correctly.

@yuri-kiss

This comment was marked as abuse.

@GarboMuffin
Copy link
Member

If it looks like the screenshots then I like it, it's cool

@Brackets-Coder
Copy link
Contributor Author

Brackets-Coder commented Mar 31, 2025

If it looks like the screenshots then I like it, it's cool

Still have a few issues and things to work out before this is ready to be merged, but I'm abnormally busy with life right now so I'll get around to it whenever I can

@Brackets-Coder
Copy link
Contributor Author

Brackets-Coder commented Mar 31, 2025

I've added a comment above the alert icons with a link to the source SVG and fixed the format issues. I'll do the "rest" after I get some "rest"
Really bad pun intended

@Brackets-Coder
Copy link
Contributor Author

@yuri-kiss the commit I just pushed should make this merge-ready

Copy link

@yuri-kiss yuri-kiss left a comment

Choose a reason for hiding this comment

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

Merging :3, great job mate

@Brackets-Coder Brackets-Coder deleted the docs-enhancement branch March 31, 2025 16:03
@Brackets-Coder
Copy link
Contributor Author

Some things still don't work... attempting to use note types that aren't all caps doesn't throw an error, eg., no warning is passed when doing > [!Note]

@Brackets-Coder Brackets-Coder restored the docs-enhancement branch March 31, 2025 16:18
@yuri-kiss

This comment was marked as abuse.

@Brackets-Coder
Copy link
Contributor Author

Some things still don't work... attempting to use note types that aren't all caps doesn't throw an error, eg., no warning is passed when doing > [!Note]

make a follow up for nitpicks

Should I make it case-insensitive so things like > [!Caution] work or should I make it require all caps so only > [!CAUTION] works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: other Pull requests that neither add new extensions or change existing ones

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documentation Improvement Suggestion

4 participants