picocms / pico-theme

This is the official default theme for Pico, a stupidly simple, blazing fast, flat file CMS.
http://picocms.org/
MIT License
11 stars 9 forks source link

Titleless pages being hidden is a paper cut. #4

Closed mayamcdougall closed 2 years ago

mayamcdougall commented 2 years ago

This was brought up in https://github.com/picocms/picocms.github.io/pull/51, but that thread has gone off topic, so I'm moving the issue here.

The behavior of hiding titleless pages in navigation is a paper cut for new users. It's not documented anywhere, and overall feels more like an oversight than an intended behavior.

From a birds-eye view, it makes sense to hide pages without titles to avoid outputting empty <a> tags.

But in practice this means that a user who forgot their title is left wondering where their page is. Also, it gives users an impression that removing your title is an intended way to hide a page (rather than using Hidden, which isn't actually documented at the moment, as far as I can tell).

While this could be a simple addition to the Docs (https://github.com/picocms/picocms.github.io/pull/51), telling users not to forget their title, I don't really feel like this was an intended behavior in the first place. Rather, it seems like an oversight of how it could affect users (because again, there's already an intended way to hide pages, so this behavior feels confusing).

The simplest way to change this could be to fall back on using page.id when there's no title. A ternary statement (? :) could accomplish this with almost no effort, but it does add complexity to the readability of the code. Since readability is a high priority for the Default Theme, I'm a little torn on this, but it seems like the easiest option.

{% for page in pages(depthOffset=-1) if not page.hidden %}
     <li{% if page.id == current_page.id %} class="active"{% endif %}>
          <a href="{{ page.url }}">{{ page.title ? page.title : page.id }}</a>
     </li>
{% endfor %}

Avoiding the ternary statement would require adding a little bit more code, but might be more readable for a new user:

{% for page in pages(depthOffset=-1) if not page.hidden %}
     <li{% if page.id == current_page.id %} class="active"{% endif %}>
          <a href="{{ page.url }}">
               {% if page.title %}
                    {{ page.title }}
               {% else %}
                    {{ page.id }}
               {% endif %}
          </a>
     </li>
{% endfor %}

But either way, I'm leaning more toward changing this behavior itself than adding a warning to the Docs describing a behavior specific to the Default Theme, that both isn't part of Pico's core, and doesn't affect most themes (at least, the ones that we have at the moment).

@PhrozenByte I'm more than happy to make whichever changes, just wanted to get your input first.

mayamcdougall commented 2 years ago

Unsubscribing from this now (didn't read anything of it), please go ahead, if there's anything relevant for me in here just let me know in separate issues 👋

@PhrozenByte Already did, it's over here. 😉

Edit: Also, sorry that that's probably been blowing up your email all week. 😅

PhrozenByte commented 2 years ago

@PhrozenByte I'm more than happy to make whichever changes, just wanted to get your input first.

Yeah, go ahead :+1:

By the way, you can shorten the ternary by using {{ page.title ?: page.id }}.

mayamcdougall commented 2 years ago

Holy replies! Welcome back, lol. 👋🏻

By the way, you can shorten the ternary by using {{ page.title ?: page.id }}.

I didn't know that! 😮

That would make a lot of the ternaries I used in Freelancer a lot simpler too! Way less cumbersome, lol.

Also, I'm currently writing a Docs page for the rewrite that breaks down the Default Theme into bite-sized, plain text explanations. So, any concerns I had about the ternary syntax needing an explanation will be addressed there too. 😁

mayamcdougall commented 2 years ago

Alright, that was scary but cool.

I know I shouldn't commit directly to master, but that was super easy.

(Well, minus the crippling anxiety about how I'll probably notice a mistake in 30 seconds and have to issue another commit... then another to fix that... and only then will I still not learn that committing directly to master is a bad idea. 😂)

Anxiety aside. I used GitHub's new Dev feature to make the changes in about 10 seconds. It took me longer to internally freak out over the commit (👆🏻) than to actually make the change.

And yeah you could do it using the regular GitHub edit feature already... but it's kind of garbage at being a code editor.