naturalcrit / homebrewery

Create authentic looking D&D homebrews using only markdown
https://homebrewery.naturalcrit.com
MIT License
1.09k stars 326 forks source link

`undefined` css code blocks in /source #1846

Closed ericscheid closed 1 year ago

ericscheid commented 2 years ago

If a brew doesn't have any css added via the editor panel, the brew when viewed via /source will have a css code-block prepended that looks like this:

```css
undefined


That's a bug, easy to fix.

Also appears in the `/download` resource.
Gazook89 commented 2 years ago

If no document styles are specified, should there be no style block at all? Or should it show as:

```css
No document-wide styles defined  (or alternate message)
```\

Unrelated, but how did you get code fence to show in github (how to escape the grave accents)?

calculuschild commented 2 years ago

I think it should be blank or not there at all. Filling it with other text adds an extra step to determine if it's actually empty.

Markdown can also display code by indenting the whole block four spaces which let's you escape the backticks.

Gazook89 commented 2 years ago

Agreed, I couldn't think of a good message to go in that spot anyway--- just because it's not entered int he Style editor doesn't mean there isn't "document wide styles" in the main content via style tags.

Okay, i may pick this up this weekend and try my hand (however, if anyone beats me to it I won't be upset :smile: -- I know Jeddai is doing related stuff )

ericscheid commented 2 years ago

image

calculuschild commented 2 years ago

Reading this issue again I realize I misunderstood what part of the code this was referring to. undefined here is not a bug, but intentional because we need to differentiate between two states:

1) The user has never touched the Style tab for this document, in which case we need to display some default filler text. If this is the case, we put undefined.

2) The user has touched the Style tab and intentionally deleted all of the sample text to have the Style tab blank. If this is the case, we just have the CSS block blank. We don't want default text coming back every time the user deletes the contents of that tab.

I can see how this would be confusing though, and so perhaps the better solution is to entirely remove the block to represent undefined. Then, on loading, it is easy to see whether any style data exists and whether to inject the default text.

calculuschild commented 2 years ago

Actually, I take that back. You're right, we don't deliberately inject the string "undefined" anywhere so proper behavior should be to not have the CSS block at all if the the style tab was never touched.

Gazook89 commented 2 years ago

So #1850 does this:

  1. If style tab isn't touched from "new", the example styling doesn't appear, and no code fence block.
  2. If it's changed, the change appears
  3. If it's changed, but entirely deleted, it shows the code fence block but is empty:
```css
calculuschild commented 2 years ago

@ericscheid Can you provide specific steps to reproduce this? The only way I can think to get this result is viewing older documents made when we were first getting the Style tab working and ironing out the behavior (in which case there's not anything we can do). New documents do not have this behavior that I can see.

ericscheid commented 2 years ago

@calculuschild testing just now and I concur. It must have been an old brew I was looking at. What we can do is make sure we don't consume the string "undefined" from an old brew and stick that into the style editor.

Ah, found these two in my downloads folder:

https://homebrewery.naturalcrit.com/share/HJ-xEkR-77 https://homebrewery.naturalcrit.com/share/1Xy5P5f_d4aYQ8mSSTATBQtKuOLqI8Qs7N7hF0h2ml8Gf

Since this is a minor glitch of bad data that won't cause any problems, we can safely ignore.

calculuschild commented 1 year ago

Closing, since I think we are in agreement that since this is only a leftover from certain old brews, but doesn't cause any noticeable issue other than an oddity in the source results.