climateaction-tech / branch-theme

The theme used in the branch magazine by climateAction.tech
13 stars 2 forks source link

Footer improvements #35

Closed jacklenox closed 1 year ago

jacklenox commented 1 year ago

As per our card on Trello, tasks for this PR:

This is now all set up on the staging environment with provisional content.

One note: I remembered in doing this that how I've usually tackled the sitemap/page visibility issue is to publish the 'Footer' page privately. This means the content is still accessible in the code to retrieve it, but isn't included in the sitemap and can't be found here: https://branch-staging.climateaction.tech/footer/ (unless you're logged in, of course)

jacklenox commented 1 year ago

Am I right in looking at the code that you've enabled the upload of SVGs to the media library? I have mixed feelings about this, especially given the fact we often a rotating band of editors adding content to the site. I know there are some risks with uploading SVGs which is why this is disabled by default. How do you rate the risk with regards to security and also to the carbon aware approach? eg I don't think svgs will get the carbon-aware image treatment (which might be fine, but would value your thoughts!).

@hanopcan Good catch! Yes, you're absolutely right and this is a very valid point. I went with my go-to solution here for my own projects, but these typically only involve me as the admin. On double-checking some other client projects, I've realised I normally use 10up's Safe SVG plugin. This is great because it offers some solid safety checks of SVGs, but more importantly, includes an easy way to administer which users can upload them. So I'd say let's definitely opt for that.

On your point about our carbon-aware approach, I think it's going to be rare that we'd want to use SVGs in the editorial part of the article. We can probably just restrict SVG uploading to you and me for the moment, and we'll explicitly just use them for logos etc in the non-editorial parts of the site.

On the carbon aware approach, I noticed that all the logos in the footer were displaying by default. That's kinda cool but I was expecting to have to add the no-carbon class to make that happen. Could you help me understand how they are showing - I'm sure I've missed something along the way!

Also a good question. The carbon-aware image treatment is restricted to the .hentry part of the website. From the outset, this was to prevent it interfering with things like the Branch logo and ClimateAction.tech logos in the header. Fortunately, it also means our new footer “just works”. I think this is desirable because the footer is not like any other editable content we currently have on the site.

However, thinking about what you've said about SVGs and security, I wonder if we should hide the footer from the admin so it doesn't cause confusion for anyone else looking at it. This would also prevent it being accidentally tinkered with or misunderstood by the rotating band of editors. We could either restrict it to certain users, or just operate knowing that it's hidden, and we have a shared direct link on a need-to-know basis to edit it. The latter is quicker and easier, but technically an editor could make changes if they have the direct link.

jacklenox commented 1 year ago

Sorry to pull you back in @hanopcan, but my latest commit auto-dismissed your review. If you wouldn't mind just giving that last commit a quick check and (hopefully) an approval, that'd be much appreciated!