Theming Discussion - Githubissues

@PhrozenByte don't feel the need to read or answer this all at once. I just didn't see the point of making multiple issues on very similar topics.

I've been laser focused on updating an old theme this week (Freelancer). It's gonna be pretty good. 😉

But, in the process I've had a few issues I wanted to discuss, especially considering the pico-theme.yml standard (and our as far as I can tell lack of documentation on it). (Okay, I lied here, it was like one question. 😂)

I've had a bunch of random thoughts and questions related to theming as I've been going, but I figured I'd hold them until the end and see what I couldn't just answer myself.

This is just meant to be a thread for a general discussion, not a singular issue. So, let's get into it.

1) The pico-theme.yml's meta-headers section doesn't seem to support arrays for having multiple levels of depth. Not sure whether this is an oversight or by design. The use case I'd describe here would be:

I have several meta-headers named portfolio_title, portfolio_divider, portfolio_nav and portfolio_disabled.

These could be written:

Portfolio Title: Recent Projects
Portfolio Divider: clock
Portfolio Nav: Projects
Portfolio Disabled: false

But I feel they'd look better as:

Portfolio:
  Title: Recent Projects
  Divider: clock
  Nav: Projects
  Disabled: false

It's fine if you've decided you just don't want Pico to have multi-level arrays of metadata, but I have been using them to simplify the YML on my themes for quite a while, lol. 😂

What are your thoughts on this? Is it an idea you absolutely hate or might it have potential?

Obviously in the meantime I can still accomplish this without registering them in pico_theme.yml (that's how I've been developing it anyway), they'll just have mandatory lower-case names like portfolio > title.

On a related side note, I almost opened this issue a few days ago to discuss how weird and inconsistent it feels when, if you don't register meta headers, they behave somewhat opposite of the built-in ones:

We typically write the meta headers with the first letter capitalized. When you do this, it then only presents itself as lowercase in Twig (eg Title becomes title). But if you have an unregistered custom header, the opposite becomes true. You write it with the first letter capitalized and it is only available the way you wrote it (Image stays Image).

I know why this is the case... it just feels odd and like it could be something of a trap for new users.

2) I've noticed that on some of my old themes (that I haven't got around to fixing yet), whitespace has started being removed by Twig. This has led to text beingcrushedtogether in some cases. Just curious if you knew when and why this behavior changed (besides the obvious "sometime between Pico 1 and 2 😂).

2) Is Twig's raw filter generally "safe" to use in Pico? I've been using it on this project more than I intended too because if I do anything to manipulate strings before printing them they become escaped, which doesn't work so well for HTML tags. 😅

It doesn't really seem like an issue considering our usual attitude toward security (that any access to Pico's files is the same level of security, and if someone had access to put something malicious in the markdown anyway, then they could probably already do a lot worse without caring about the Twig template, etc).

3) Pico's markdown filter doesn't seem to throw errors. This is a very different behavior to Twig's built in filters (which complain. A lot.). I accidentally fed it an array a couple times and it just silently fails. Compare this to Twig throwing an "Array to string conversion" notice every time I accidentally forget to json_encode my debug code (🤦🏻‍♀️), and I just feel like it should say something when you do it wrong. No idea if Pico's other filters are like this or not.

Do you think they should print errors for debugging?

4) It seems like having a _meta.md file is now an "officially endorsed" concept (at least in the docs). A lot of themes (Freelancer included) ignore index.md in favor of giving a custom front-page (or single-page) experience.

I like the idea of having theme-related/site-wide metadata in _meta.md (and it's what I've used this time), however, it does leave an awkward index.md-sized hole to be filled. 😉

Specifically, I mean you still require an empty index.md in this case, even if you're not using it, both for Pico's content folder detection and for preventing 404's (with the content dir manually specified in config.yml).

I guess this is mostly an opinion thing, but it leaves me wondering what approach is better in this case. If I use index.md for the metadata, it's at least used for something. The instructions wouldn't need a step that says "make sure you have an index.md, even if it's empty". What do you think? Would you rather themes suggest a blank index or that they forgo _meta.md in favor of sticking everything in index.md?

5) 404-ing on _meta.md behaves weird.

So, this theme rewrite is heavily dependent on _meta.md containing all its variables (don't worry, I've hard-coded defaults, so it all looks very professional even with an empty _meta.md 😉). I decided to test that Pico supposedly denies access to _ files, and I discovered an odd behavior. When it 404's on trying to access _meta.md... it also denies access to all the metadata I was accessing on _meta.md, causing my very customized site to revert to the demo data I've included.

Presumably this is happening because there's a shortcut somewhere in Pico that goes "I need to access this page. Oh, good, I'm already on it, I'll just look at current_page instead." Except its current page has been redirected to the 404 page. 🤦🏻‍♀️

Just kind of an odd quirk. I don't need it fixed or anything, just thought I'd bring it to your attention. 😂

6) Kind of surprised that using pages.pageid instead of pages["pageid"] still works. I'm assuming this is still just a legacy, compatibility behavior, since I remember you telling me a long time ago that it was preferred to use pages["pageid"] instead. I guess that's all, it was just kind of a "Huh... neat" moment. 🤷🏻‍♀️

7) This one should probably be a separate discussion entirely. Do you have any good suggestions for maintaining a large patch set against an upstream? The Twig bits I've written for Freelancer are exhaustively extensive. I've spent over a week on it now (while the Themes Gallery project hangs in limbo 😒). It just keeps going, lol. I need to stop adding things to it.

The original theme is written with Bootstrap, in SCSS and PUG. Since the Pico port is based on the compiled version, I can't exactly just grab the original repo as an upstream (though, maybe I could potentially pull from their compiled dist folder somehow).

At the moment, I plan to remove the upstream on my existing port (it was based on an out-of-date Jekyll version), and just replace the entire repo with the new version and sort this out later.

For future updates of Bootstrap and Freelancer though, it'd be nice to have a solution in place to be able to bring the changes forward without essentially starting from scratch every time.

At the moment, the best idea I'd have would be to diff the old and new Freelancer versions and simply use that to get a rough idea of what changed between versions. Or, you know, see if I could somehow make a patch file based on the diff between my version and theirs, but I think the changes might be too extensive for that (or at the very least, I might have to merge all my includes back into one mega-template). Either way, I don't see it being as easy as "fetch and merge upstream". 😩

So, if there's a fairly obvious solution you think I should look into, let me know. Not looking for a full solution, just a nudge in the right direction if you have any ideas. 😅

Alright, this has gone on long enough. I'm so ready to move on from this theme, but I keep finding one more thing to do.

And I promise the next one will be a "regular" theme. This one's just been needing some attention. 😉

Oops, rolled the Issue tracker over to 600. 😅

Damn, should have come up with something better for that. 😂

Hey, at least it wasn't an Nginx issue this time! 🤣

The pico-theme.yml's meta-headers section doesn't seem to support arrays for having multiple levels of depth. Not sure whether this is an oversight or by design. The use case I'd describe here would be:

Yes, it doesn't support arrays right now. That's mostly because I'd rather consider this being sort of data validation, what is out-of-scope. The idea behind registering meta values is to allow aliasing meta values - like the mentioned Portfolio vs. portfolio (but not limited to, you can also alias Portfolio to foobar if you want to).

Anyway, I totally agree that this is inconsistent - and it's quite hard to fix this inconsistency. To be honest I'd rather go for removing aliasing altogether. If a theme expects portfolio, users should use portfolio and not Portfolio. We shouldn't mess with a user's meta values...

I've noticed that on some of my old themes (that I haven't got around to fixing yet), whitespace has started being removed by Twig. This has led to text beingcrushedtogether in some cases. Just curious if you knew when and why this behavior changed (besides the obvious "sometime between Pico 1 and 2 joy).

No idea, AFAIK they didn't change anything there. Did you check the HTML sources or just how it is being displayed? Maybe it's just an browser issue?

Is Twig's raw filter generally "safe" to use in Pico? I've been using it on this project more than I intended too because if I do anything to manipulate strings before printing them they become escaped, which doesn't work so well for HTML tags. sweat_smile

Yes and no. One should avoid using Twig's raw filter whenever possible, because it might introduce security issues with Pico CMS for Nextcloud (which I'd consider equally important - not for Pico itself though). Usually there's a better solutions than Twig's raw filter.

Pico's markdown filter doesn't seem to throw errors. This is a very different behavior to Twig's built in filters (which complain. A lot.). I accidentally fed it an array a couple times and it just silently fails. Compare this to Twig throwing an "Array to string conversion" notice every time I accidentally forget to json_encode my debug code (🤦🏻‍♀️), and I just feel like it should say something when you do it wrong. No idea if Pico's other filters are like this or not.

Indeed. Can you open a separate issue for this? I'll add some checks later then.

It seems like having a _meta.md file is now an "officially endorsed" concept (at least in the docs). A lot of themes (Freelancer included) ignore index.md in favor of giving a custom front-page (or single-page) experience.

What's the reason for these themes to ignore index.md? Even if it's a single-page theme, why isn't index.md used to manage contents? Just take clean-blog for an example: It doesn't show the contents of index.md either, but honestly I don't really see a reason why. The theme could still show the contents right above the article list.

404-ing on _meta.md behaves weird.

Indeed, definitely a bug. Not sure why this is happening. Can you open a separate issue for this? I'll check this later then.

Kind of surprised that using pages.pageid instead of pages["pageid"] still works. I'm assuming this is still just a legacy, compatibility behavior, since I remember you telling me a long time ago that it was preferred to use pages["pageid"] instead. I guess that's all, it was just kind of a "Huh... neat" moment. 🤷🏻‍♀️

Hmm... Not sure what you mean? This is Twig syntax - and Twig supports both and AFAIK there are no plans to remove this. It just might cause confusion, because the dot-syntax has some limitations regarding the keys supported (e.g. no whitespaces, one can't use a variable for the key)

This one should probably be a separate discussion entirely. Do you have any good suggestions for maintaining a large patch set against an upstream? The Twig bits I've written for Freelancer are exhaustively extensive. I've spent over a week on it now (while the Themes Gallery project hangs in limbo unamused). It just keeps going, lol. I need to stop adding things to it.

I'd recommend to let Git do this job for you. Use the original SCSS as a basis and add your changes. Choose solutions with which you don't have to edit the original SCSS sources too often. Download a SCSS compiler and also commit the compiled CSS to your repo. Rename index.html to index.twig first and then apply your changes step by step, so that Git still knows that index.twig is index.html and tracks changes. If you later wanna update from upstream, simply do a pull. You'll likely end up with some conflicts, but it should remain manageable - e.g. you can simply ignore conflicts in the compiled CSS.

Sorry in advance for not addressing these in order, but I'm gonna bounce around a little as I get to each issue today.

No idea, AFAIK they didn't change anything there. Did you check the HTML sources or just how it is being displayed? Maybe it's just a browser issue?

I found an example of this to share. From the old Freelancer port:

The output, currently displayed on my personal site w/ Maya McDougall as site_title, and no pages.index.meta.date (so it's just using the current date):

For some reason, all the whitespace directly surrounding Twig's print tags is removed (And yes, it's actually written in the HTML like that, it's not a browser or style issue). This didn't happen back when I wrote the theme (otherwise, I wouldn't have left it like that!), and I've seen it exhibited in a couple of others too.

This is likely a Twig issue, not a Pico one. I was just curious if you had any insight into it. The only info I can find involves Twig changing how it handled newlines at one point, but nothing about spaces.

Obviously this can be worked around in a couple ways (concatenating a space into the string, using an nbsp instead, etc etc).

It's just odd that it changed at some point. 🤔 🤷🏻‍♀️

Issues made. 😉

Sorry for giving you a todo list. 😂

Yes, it doesn't support arrays right now.

Anyway, I totally agree that this is inconsistent - and it's quite hard to fix this inconsistency. To be honest I'd rather go for removing aliasing altogether. If a theme expects portfolio, users should use portfolio and not Portfolio. We shouldn't mess with a user's meta values...

I'd almost wonder if it'd be better to just standardize on lowercase names and convert all the keys to lowercase at runtime. I honestly don't care that much about the ability to alias things, but lower vs upper case feels like a usability issue. To a newer user, the idea that some meta properties can be upper or lower case (the internal ones), while some must be a specific case feels like a paper cut.

There's a small chance that changing this behavior could break older themes, not that I know of any that use uppercase Meta headers, but they could be out there. This could probably be mitigated by only applying the new behavior to themes with a certain pico-theme.yml api version.

I'm just brainstorming though. I realize this is all kind of a niche case because your current solution of aliasing them solves like 90% of the problem. I just tend to go a little crazy when adding customization to a theme. 😉

Usually there's a better solutions than Twig's raw filter.

Agreed. I've worked around most of the places I was using it. It just really nerfs Twigs capabilities when most of its filters escape their output. The areas where I was using raw were instances where the output would have printed as-is before I filtered it. But, I can't say for certain that something clever couldn't exploit that in the right circumstances, so... 🤷🏻‍♀️

One place I couldn't find a good workaround though was adding bootstrap classes to my markdown content paragraphs:

{{ content|replace({"<p>": '<p class="mb-4">'})|raw }}

I should just be a be able to throw a style on the parent container to replicate this, but I was trying to keep consistency with the original theme anywhere I could. 😅

If I'm being honest with myself, a dedicated style would be the more "proper" solution anyway, I've just been being stubborn about it. 😒

(I've mentioned it a lot in this post, so here's a link to the StartBootstrap Freelancer demo site for a visual reference.)

What's the reason for these themes to ignore index.md? Even if it's a single-page theme, why isn't index.md used to manage contents? Just take clean-blog for an example: It doesn't show the contents of index.md either, but honestly I don't really see a reason why. The theme could still show the contents right above the article list.

In general, I'd say it's a design decision. Especially when working with an existing theme or template, shoehorning in a content section just to have it feels sloppy. Obviously, it depends on the situation, but many of these one-page themes or themes with dedicated front-pages just didn't originally have a space for general content.

The theme could still show the contents right above the article list.

As someone who sits somewhere between the "Designer" and "Developer" camps, this reads like the most Developer-y thing ever. 😂 😜

(I'm very much teasing there, and I hope that's clear, lol.)

I mean, yeah, you could just shove it somewhere... but please don't! If there's just not a good place for it, where it can look intentional and fit in well, I'd personally rather see it omitted. 🤷🏻‍♀️

I just don't feel like it should be mandatory for a theme to carve out a spot for content if it's already doing something else to fill out that page.

As for Freelancer, the sections of the page alternate between having a background color and having a white background. Anywhere I could put content would feel off, since it would have to break this convention. That being said, I do allow for sections to be disabled which also breaks the color convention (🤔), but this isn't the default behavior.

I think that's where most of my problem with it is. If a theme isn't really meant to have a content spot, and you just put one somewhere... A user's first impression of the theme will be something that looks wildly different than the picture because either their content or Pico's sample content is breaking up a large portion of the page that wasn't necessarily meant to hold any extra content!

It also suggests to the user "Hey, you should put something here", even if your intention was that most users would leave it blank.

This is all just my rationale though, I don't expect you to agree with it. 😉

Even if it's a single-page theme, why isn't index.md used to manage contents?

I've also tried to use index's content in the past to fill out certain specialty blocks of text, but it always feels clunky. The user experience is like "Okay, so this block of arbitrary text is your index's content, while this one comes from a Meta header and this one is your site_title. It gets confusing quickly.

This time around I've tried to do a lot better in my port than just "replace the strings" using Pico. 😂

There are two blocks of freeform text in Freelancer (technically three if you count the "Location" block that's meant for an address, since I made it freeform this time): The regular About section, and the Footer's About block.

While I could make one of these just use content, neither actually hold enough text to make that feel like a good solution. I've kept them both as meta headers, though they are named about_content and footer_content to keep with Pico's terminology. They also support markdown, and multiple paragraphs, so they're each very much acting as a mini content.

There isn't going to be a place for index.md's content, however, I am adding regular content-page support to the theme if you navigate to any non-index page! They're not going to be exposed anywhere as links or navigation, but the option will exist if someone wants to, for example, link one of their Portfolio items to a more in-depth page. 😉

Ultimately though, I really don't like the idea of an empty index.md either.

I think I'm going to move all the site metadata from _meta.md to index.md. I mean, it is essentially the "content" of the index page, even if it's not the content of the index page. 😉

And I promise to make the next theme a "normal" one that uses Pico the way you want to see it used. 😇😅

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in two days if no further activity occurs. Thank you for your contributions! :+1:

@PhrozenByte This is absolutely the lowest priority right now, and I don't want to add more to your plate when you get back.

Save this thread for when you have time. If that's never, that's perfectly fine too. 😉 (Though I may start doing things without asking if that's the case. 😜)

So, finally, two months later I've finished the port I thought I'd have done by the end of the week.

To be fair, 99% of it was done, then I got hung up making the damn readme! 🤦🏻‍♀️

It's HUGE!

But that's finally done, and I made some usability improvements along the way too.

I think you'd overall approve how it turned out. Don't worry, there's no more empty index.md, I got rid of _meta.md and used the index page instead.

Literally the only problem you should have with it is that it doesn't use index.md's contents. Sorry (not sorry *cough*), it just doesn't fit with the theme. There's plenty of other places to put text on the main page without it. 😉

If you get some time to check it out (mayamcdougall/freelancer-pico), tell me what you think.

If it's up to your standard, I'd like to offer it as the first (non-default) "Official" Pico theme.

It's new, fresh code. It's highly documented. And I'm always around answering questions here anyway, so it's not like an extra burden or anything. I would still plan to redirect any issues that arise to its own repo. It would still be considered a separate project, even if it's "Officially" supported.

What do you think?

I'd like to obviously continue this pattern with several Officially supported themes. Especially ones that use Pico in a more traditional manner (just to make you happy though 😉).

I don't think I would necessarily do this right away, but maybe when I have another option or two ready.

At some point I'd also like to revamp my older stuff to live up to my current expectations. Honestly, I'd probably label all my themes as "Official" once they were up to par. I'd like users to have a good selection of ready to go themes they knew they could ask questions about. Hopefully I could eventually distance us from the "contribute and forget" model we have now, but that's probably wishful thinking. 😅

@mayamcdougall - amaizing work with port. I just looked and a big thanks for such contribution to Pico CMS. I am new guy around, meeting 'nginx; problems but I have finger crossed for Pico.

Thanks, lol. 😁

Just finished answering your other question. 😉

Nginx can be a paaaaaiiiin. Hopefully you've got it working okay.

I might be the person who wrote our Nginx documentation originally, but that does not mean I remember enough about the finer details to fix a problem. So I'll be keeping my fingers crossed for you there. 🤞🏻😂

Again, don't be afraid to ask any questions you might have about Pico. I can usually answer questions faster than our documentation can. 🤣

(Sad, but often true. I'll get it rewritten for everyone... eventually... 😓)

I'd almost wonder if it'd be better to just standardize on lowercase names and convert all the keys to lowercase at runtime. I honestly don't care that much about the ability to alias things, but lower vs upper case feels like a usability issue. To a newer user, the idea that some meta properties can be upper or lower case (the internal ones), while some must be a specific case feels like a paper cut.

The problem is that this is going to be inconsistent with arrays (i.e. {{ meta.some_array.SomeValueThatRequiresUppercase }}). For Pico 3.0 (probably Pico 4.0 due to the dependency issues...) I was thinking about switching to objects to represent pages. We could do the same for meta data, allowing us to access all meta values no matter the depth case insensitive by default... I'll add this to the ToDo list for Pico ~3.0~ 4.0.

One place I couldn't find a good workaround though was adding bootstrap classes to my markdown content paragraphs:

{{ content|replace({"<p>": '<p class="mb-4">'})|raw }}

You shouldn't. Rather leave it a regular <p> and wrap it inside a literal <div> in your theme. You can then either apply the mb-4 class to that div instead or add additional CSS/LESS/SCSS rules to interpret all <p> below this <div> as mb-4.

In general, I'd say it's a design decision. Especially when working with an existing theme or template, shoehorning in a content section just to have it feels sloppy. Obviously, it depends on the situation, but many of these one-page themes or themes with dedicated front-pages just didn't originally have a space for general content.

Agreed. If there's really no place for {{ content }}, leave it.

We might add a new config section to pico-theme.yml to configure "static" pages (added this to the ToDo list) that don't require a Markdown file to exist to not yield a 404 page. However, requiring index.md to exist is a valid exception in my opinion.

I think I'm going to move all the site metadata from _meta.md to index.md. I mean, it is essentially the "content" of the index page, even if it's not the content of the index page. :wink:

:+1:

I've also tried to use index's content in the past to fill out certain specialty blocks of text, but it always feels clunky. The user experience is like "Okay, so this block of arbitrary text is your index's content, while this one comes from a Meta header and this one is your site_title. It gets confusing quickly.

I often use separate files for this, e.g. the page foo.md comes not just with the main foo.md, but also includes a _foo/box1.md, _foo/box2.md, etc. to represent pre-defined containers in my theme. I often restrict meta values to short single lines (e.g. headlines). For contents spanning multiple pages (e.g. a footer) I often use e.g. a _footer.md.

It's new, fresh code. It's highly documented. And I'm always around answering questions here anyway, so it's not like an extra burden or anything. I would still plan to redirect any issues that arise to its own repo. It would still be considered a separate project, even if it's "Officially" supported.

What do you think?

Unfortunately I don't have much time to do a "deep-dive" here, but everything I saw looks very, very good. Nothing tripped me. Please don't interpret my notes above (regarding the use of the raw filter, or using the markdown filter for multiple lines, etc.) as criticism, there's no single "right" answer, there are always multiple valid and absolutely equal solutions. Very extensive docs, great! :+1: I really love your work. Awesome! :heart:

If you choose to add an "Official" tag to themes I'm totally fine with it, I'm very confident in your considerations :smiley: :+1:

I was thinking about switching to objects to represent pages.

Interesting 🤔

I don't know enough about PHP to really comment on it on a technical level, but from an "any other language I know" perspective, that does sound like it'd be a lot easier to work with on the backend. 😉

You shouldn't.

lol. I was just being really OCD about keeping the output HTML the same as the source. 😅

I did end up styling it with extra CSS instead.

Don't worry, all raw filters were removed by the end. 😂

Agreed. If there's really no place for {{ content }}, leave it.

I didn't expect you to budge on that opinion. 😱

However, requiring index.md to exist is a valid exception in my opinion.

I agree. I liked the idea of putting things in a special file like _meta.md. It felt less "hacky" that way. But without another use for index.md, it would never have been a good user experience. I'll just have to use _meta.md for something next time.

I often use separate files for this

Wait wait wait, hold up. You use Pico? And here I thought you just hacked on it once in a while. 😂

I'm kidding obviously, but you do very seldom talk about how you actually use Pico.

I like that idea of having chunks like _foo/box1.md and _foo/box2.md. For a bigger, non-one-page Theme, I could see that being a really powerful way to keep things organized. Might even be a good strategy for the Pico site in the future. Keep all the modularity it has with Jekyll right now, but clean out all the clunky Jekyll cruft. 😉

Unfortunately I don't have much time to do a "deep-dive" here

No worries, I didn't expect you to. I didn't actually think you'd continue the earlier discussion here either. I appreciate it, it's just a little obsolete now.

Please don't interpret my notes above as criticism, there's no single "right" answer

No worries again. I was actually able to make the code much cleaner by removing the raw usage anyway.

I had gone down a bit of a rabbit hole of using some clever string manipulation to change which tags I was writing. It was super super clever... and entirely unreadable. You know you've gone in the wrong direction when you* have trouble re-reading your own code.

(* Hey, another of those expressions. See what I said about the Docs rewrite comment.)

I did still end up with a clever markdown_by_paragraph macro that I used to turn multi-line YML strings into separate markdown paragraphs (without having to double-space the paragraphs because that makes the YML look bad in my opinion, lol). Totally over-engineered, but it also allowed me to customize those <p> tags depending on the situation. 😂

If you choose to add an "Official" tag to themes I'm totally fine with it, I'm very confident in your considerations 😃 👍

Thanks. 😁

It's a future concern anyway. It would be odd if the only two "Official" themes were the Default Theme and Freelancer. There's a LOT of difference between the two (you know, like, everything). So until there's more options, I'd rather not push new users toward a theme that works so differently from the Default Theme.

picocms / Pico

Theming Discussion #600