Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor front and back end issues #507

Closed
LuisSR opened this issue Aug 18, 2023 · 4 comments
Closed

Minor front and back end issues #507

LuisSR opened this issue Aug 18, 2023 · 4 comments
Assignees

Comments

@LuisSR
Copy link

LuisSR commented Aug 18, 2023

After some days of testing the new version of Flynt, I have detected a few small issues in the default implementation of the theme.

I don't have enough information about how you work with the theme on client projects. I guess some of the things might not be issues at all but the intended behaviour. Or maybe just code ready to be customized and not taken literally.

Either way, I don't feel confident enough to push any change yet. Some of the solutions I propose require discussion and testing.

This might be a very long issue, but I'll try to structure it the best I can.

I've described each of the issues and provided a fix or the starting point for creating one. I can help you with time-consuming fixes as long as client work doesn't get too busy again.

Unless stated otherwise, all the code has been tested on localhost with WP 6.3 running Flynt 2.0.0 and ACF 6.2.0 over PHP 8.2.4. The browsers are either Firefox 116.0.2 or Opera 101.0.4843.43 under Kubuntu 23.04.

1. Back end issues

1.1. The theme can be activated in servers with a PHP version lower than 7.4 and leave the WP installation unusable

Description

Activation using an outdated PHP version will render a Composer related fatal error both on the dashboard and on the frontend of the site.

Although it's unlikely to happen that a non-technical user downloads and installs Flynt, the fix is so easy I see no reason not to implement it

Possible fix

Just add the requirement as /style.css headers:

/*
    Theme Name: Flynt
    Theme URI: https://www.flyntwp.com/
    Description: The lightning-fast WordPress Starter Theme for component-based websites with ACF Pro.
    Version: 2.0.0
    Author: Bleech <hello@bleech.de>
    Author URI: https://bleech.de/
    Text Domain: flynt
+   Requires PHP: 7.4
    License: MIT
*/

1.2. Add component button is not working as expected

Description

In the post edition screen, the "Add component" button is sometimes failing to open the tooltip on the correct position.

This has already been pointed out in the following discussion: #499.

It seems to happen when one or more of the components meta boxes are open.

Possible fix

It seems to be a simple tooltip positioning error somewhere in the JavaScript calculations. No errors are shown in console. @aaronmeder provided a working solution, which I have not tested myself: #501


1.3. The custom post type boilerplate is showing the ACF components interface, but components are not working on the frontend

Description

The title pretty much summarizes the issue: when a new post type is registered following the code provided in /inc/customPostTypes/example.php, the post edition screen shows both the TinyMCE and the Flynt component edition interfaces. However, content written with the latter is not shown in the frontend.

To provide some context, I've seen that:

  • The single.twig template handles the view by default, so post_content is used and components won't be retrieved from post meta data.
  • The block editor is not enabled on the arguments passed to register_post_type().

This falls back the editor to TinyMCE, which probably offers the worst experience out of the three possibilities.

I also believe that just showing the active editor, whichever it is, and disabling the other two, would be a good UX practice here.

Possible fix

If it were up to me, the default editor for non-componentized post types would be the block editor. It would make a more consistent experience, as this is the way blog posts already work.

A single line addition in /inc/customPostTypes/example.php would do so:

        'show_in_menu'          => true,
+       'show_in_rest'          => true,
        'menu_position'         => 5,

Hiding the ACF components interface could be done by making /inc/fieldGroups/pageComponents.php more restrictive by default:

        'location' => [
            [
                [
                    'param' => 'post_type',
-                   'operator' => '!=',
+                   'operator' => '==',
-                   'value' => 'post',
+                   'value' => 'page'
                ],
-               [
-                   'param' => 'post_type',
-                   'operator' => '!=',
-                   'value' => 'reusable-components'
-               ],
            ],
        ],

This needs proper testing.


1.4. Some printed variables via Twig need escaping

Description

Components that print database values directly into the browser always pose a risk of XSS.

Some templates and components output titles and other string values which should be properly escaped:

flynt-v2.0.0-escaping-twig-templates.mp4

Possible fix

I'd suggest reviewing each .twig file individually and going case by case.

Timber doesn't support esc_attr() for escaping tags' attributes, which could come handy, and twig's escape('html_attr') is not an exact replacement. Maybe Timber or Flynt can be extended to mimic it.

I was thinking also about how to implement a long-lasting automatized solution, but no tool seems to cover Twig escaping. There is a Linter in Symfony and a kind of CodeSniffer for Twig, but it's a non-maintained package.


2. Front end issues

2.1. Support for Safari v14 and lower is not complete

Description

Safari 13 and 14 don't support some modern CSS properties which are used for core parts of the theme, like the masthead, being the most notable padding-block:

caniuse-padding-block

Devices using Safari 14 might include MacOS and iOS devices bought before the T4 of 2021 which have been updated on their current release branch or have not been updated at all. Even the newest version of the 14 branch has issues:

flynt-v2 0 0-safari-14 6

Devices like the iPhone 12 Pro are just two generations old high-end hardware. And pretty much any blink-based browser prior to the end of 2020 will show layout problems:

flynt-v2 0 0-safari-13 1

I am pretty conservative regarding browser support, so I understand this might not be considered an issue at all, but for me the dates and devices involved are still recent.

Suggestion

Sadly, I fear this is something the autoprefixer PostCSS plugin won't be able to handle, even if "ios 13, Safari 13" are inside the browserlist support.

A full fix is out of the scope of this issue. It would mean manually reverting many properties on quite a few selectors and affect the whole styling of the header and the horizontal and vertical spacing on several component views.

But I'll be happy to help with this task if you decide to implement it.


2.2. Posts created with the core/classic block or with the TinyMCE editor do not have their content wrapped

Description

In both cases, the issue is the same: failing to constrain and center the content.

flynt-v2 0 0-post-content-wrapping

Classic blocks are the expected result of importing html content from older WP installations or from another CMS.

Apart from potential migration problems, WordPress editors who still prefer to use TinyMCE via the Classic Editor or Disable Gutenberg plugins will face the same problem.

WordPress won't convert even perfectly clean html into blocks straightaway. There were some options available for bulk converting the classic blocks from all posts into real blocks, but none is in a working state right now.

Fix

The block editor adds inline styles for limiting the width of blocks without alignment options:

body .is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {
    max-width: var(--wp--style--global--content-size);
    margin-left: auto !important;
    margin-right: auto !important;
}

By default, WordPress injects these styles into the markup of singular templates, whether it's a page, a post or a CPT post.

But Flynt programmatically removes the so called "global styles" in /inc/blockEditor.php for any view that has no blocks inside the_content.

In the case of a post created with TinyMCE it's pretty clear that the chosen validation will fail. But the reason why the core/classic block is not triggering has_blocks() might not be that intuitive. The reality is that the classic block is actually not a block but a virtual wrapper.

Short way fix:

Change the validation performed in /inc/blockEditor.php for removing the global styles:

/**
 * Remove Gutenberg block related styles on front-end, when a post has no blocks.
 */
add_action('wp_enqueue_scripts', function () {
-   if (!has_blocks()){
+   if (is_page()) {
        wp_dequeue_style('core-block-supports');
        wp_dequeue_style('wp-block-library');
        wp_dequeue_style('wp-block-library-theme');
        wp_dequeue_style('global-styles');
    }
});

Note that is_page() assumes that only pages will have Flynt components. This might not be true for all or some custom post types on more complex projects.

Ideally, I'd suggest creating a helper function to check if a singular post has been created with components instead.

Long way fix:

Disabling global styling everywhere and hard-coding into Flynt the wrapper width and a few more things (basically spacing) that are handled by the inline styles. This is easier said than done, but will have the benefit of not inheriting any future, potentially design-breaking style that upcoming WordPress releases might bring.


2.3. Small styling issues around the post header and meta

Description

  • 2.3.1. An extra divider is shown in the post meta when a post has no categories

Although it's not possible that a post has no associated category under normal usage, this behaviour is not mandatory for custom post types. Also, some content importing tools can programmatically create posts without the default category.

flynt-v2 0 0-post-header-styles-no-category

In these cases, the divider shouldn't be shown.

  • 2.3.2. Author box and categories/reading time are not correcty aligned

flynt-v2 0 0-post-header-styles-meta-alignment

  • 2.3.3. Bleeding is missing when a post has no featured image

flynt-v2 0 0-post-header-styles-no-featured-image

Possible fix

For 2.3.1., a simple check in Components/BlockPostHeader/index.twig for existing post categories is needed:

+        {% if post.category %}
          <a class="category" href="{{ post.category.link|e('esc_url') }}">
            <span class="visuallyHidden">{{ labels.postedIn|e }}</span>
            <small>{{ post.category }}</small>
          </a>
+        {% endif %}

2.3.2. is a problem of mixing different font-sizes. I'd define a font-size property for all meta elements to inherit:

flynt-component[name=BlockPostHeader] .meta {
    font-size: var(--font-size-body-small);
}

The problem with 2.3.3. is that the bleeding is attached to the featured image component (flynt-component[name=BlockPostHeader] flynt-component[name=BlockImage] {...}). I haven't tested it, but maybe it could be in the main content section instead (.wp-block-post-content).


2.4. Refine the aria-label on components that show a grid of posts

Description

Some components output post queries into cards. The whole post card is a link ("a" tag) that has an aria-label attribute assigned to the post title.

For improved accessibility, this should start with something like "Read the article [post title]" rather than just "[post title]". This is also known for triggering a warning on the accessibility score of the PageSpeed Insight tests. If I'm not mistaken, the criterion to apply would be this one: https://www.w3.org/WAI/WCAG22/Understanding/link-purpose-in-context

The affected templates are:

  • /Components/GridPostsArchive/index.twig
  • /Components/GridPostsLatest/index.twig
  • /Components/ListSearchResults/Partials/_results.twig:

Possible fix

It's a one line fix:

-  <a class="link" href="{{ post.link|e('esc_url') }}" aria-label="{{ post.title }}">
+  <a class="link" href="{{ post.link|e('esc_url') }}" aria-label="{{_e( 'Read the article', 'flynt' )|e}} {{ post.title|e }}">

3. Suggestions

The following points are not issues at all, but performance or usability improvements you might want to consider.

3.1. The default block editor options might be overwhelming for content editors

Description

This is opinionated, but the block editor provides many options for building layouts which are not useful for blog or news posting.

On one side, many blocks, such as "core/query", "core/comments", "core/navigation", "core/archive-title" or "core/rss", are intended for complex page layouts or for the Site Editor (FSE) templates. They will never be part of any real content writing experience, and some are even not working properly in regular posts.

On the other side, WP 6.3 has almost one hundred block types, many of which serve the same purpose but with slightly different styles. Should one use "verse", "pullquote" or "blockquote"? Or a "column + text/column + image" or just "media-text" block? When this styling decision is on the hands of the client employees or the marketing content editors, posts tend to end having different visual styles depending on their authors.

Having so many useless blocks bloats the block selection screen and worsens the content editor experience.

Suggestion

3.1.1. Limit the available blocks for the post type "post". This can be done via the /theme.json file or within /inc/blockEditor.php.

I, personally, prefer to opt for the most constrained experience (negating all blocks) and then decide case by case those that will be allowed:

/**
 * Control which block types are available by default.
 */
add_action('allowed_block_types_all', function ($allowed_block_types, $block_editor_context) {
    if (!isset($block_editor_context->post->post_type)) :
        return $allowed_block_types;
    endif;

    switch ($block_editor_context->post->post_type) :
        // Blog posts
        case 'post':
            $allowed_block_types = [
                'core/classic',
                'core/gallery',
                'core/heading',
                'core/html',
                'core/image',
                'core/list',
                'core/list-item',
                'core/paragraph',
                'core/quote',
                'core/table',
                'core/video',
                ...
            ];
            break;

        // Other registered post types
        default:
            break;
    endswitch;

    return $allowed_block_types;
}, 10, 2);

But it's also possible to remove just the unwanted blocks, while keeping future core blocks support open by default:

/**
 * Control which block types are available by default.
 */
add_action('allowed_block_types_all', function ($allowed_block_types, $block_editor_context) {
    if (!isset($block_editor_context->post->post_type)) :
        return $allowed_block_types;
    endif;

    switch ($block_editor_context->post->post_type) :
        // Blog posts
        case 'post':
            $registered_blocks = \WP_Block_Type_Registry::get_instance()->get_all_registered();

            unset($registered_blocks['core/query']);
            unset($registered_blocks['core/comments']);
            unset($registered_blocks['core/rss']);
            ...

            $allowed_block_types = array_keys($registered_blocks);
            break;

        // Other registered post types
        default:
            break;
    endswitch;

    return $allowed_block_types;
}, 10, 2);

3.1.2. If blocks are limited to just a few, for improved performance and visual consistency, I'd consider dequeueing the default block editor stylesheet:

	/**
	 * Remove Gutenberg block related styles on front-end, when a post has no blocks.
	 */
	add_action('wp_enqueue_scripts', function () {
	    if (!has_blocks()) {
	        wp_dequeue_style('core-block-supports');
	        wp_dequeue_style('wp-block-library');
	        wp_dequeue_style('wp-block-library-theme');
	        wp_dequeue_style('global-styles');
+               wp_dequeue_style( 'wc-block-style' );
+               wp_dequeue_style( 'classic-theme-styles' );
	    }
	});

Base blocks, such as paragraphs or headings, will inherit Flynt's styles. But others, like core/buttons will need manual styling.

This might be a bit of work at the beginning, but it will also help to match the posts visual styling with the design system of a given site.

Alternatively, the default block editor styles can be imported from the scss files available at the Gutenberg project repository: https://github.com/WordPress/gutenberg/tree/trunk/packages/block-editor/src.

If point 3.1.2. ever makes it into Flynt or a project made around it, I'd recommend you to opt for the restrictive approach. Controlling the styles and no depending on third-party CSS is much more consistent and future-proof.

The block editor has changed the styling approach several times during these years, and many of these changes have had an impact on the frontend of the websites. This is particularly undesirable on corporate websites.


3.2. Group CSS media queries

Description

Converting this (unminified for readability)

@media (min-width: 768px) {
  .flynt-component[name='ComponentA'] .container {
    property1: value1;
  }
}

@media (min-width: 768px) {
  .flynt-component[name='ComponentB'] .container {
    property2: value2;
  }
}

into this

@media (min-width: 768px) {
  .flynt-component[name='ComponentA'] .container {
    property1: value1;
  }

  .flynt-component[name='ComponentB'] .container {
    property2: value2;
  }
}

could save some KBs for each page view on every site made with Flynt at virtually no cost.

Suggestion

There is a PostCSS plugin that can perform the grouping automatically:

https://www.npmjs.com/package/postcss-combine-media-query

I added it to the vite.config.js and then I built for production, resulting in a 3.34% smaller stylesheet. On a bigger project, with several components tailored both for mobile and desktop devices, this percentage can be as high as 15% or 20%.

One prevention: this won't run in the development environment, and moving the position of media queries implies a small risk of inconsistency if the CSS is badly structured. In my experience, after years of using this technique in production, I haven't run into a single issue. But I'd double-check!


3.3. Included assets can be further compressed

Description

From the logo to the miniatures of each block, some KB can be saved here and there:

File Original size Compressed size
/assets/images/logo.svg 916 778
/Components/BlockAnchor/screenshot.png 7411 6191
/Components/BlockImage/screenshot.png 6861 4767
/Components/BlockImageText/screenshot.png 6985 5106
/Components/BlockNotFound/screenshot.png 5354 3991
/Components/BlockPostFooter/screenshot.png 5826 4230
/Components/BlockPostHeader/screenshot.png 7710 4842
/Components/BlockSpacer/screenshot.png 8383 5802
/Components/BlockVideoOembed/screenshot.png 6643 3645
/Components/BlockWysiwyg/screenshot.png 4578 2947
/Components/FeatureFlexibleContentExtension/screenshot.png 16568 8066
/Components/FormPasswordProtection/screenshot.png 4865 2551
/Components/GridImageText/screenshot.png 7521 5430
/Components/GridPostsArchive/screenshot.png 7975 7111
/Components/GridPostsLatest/screenshot.png 7719 5292
/Components/ListComponents/screenshot.png 6943 4334
/Components/ListSearchResults/screenshot.png 5052 3551
/Components/NavigationBurger/screenshot.png 8466 5038
/Components/NavigationFooter/screenshot.png 4519 3638
/Components/NavigationMain/screenshot.png 8077 4683
/Components/ReusableComponent/screenshot.png 22934 11818
/Components/SliderImages/screenshot.png 7299 4527

It doesn't make much of a difference (about a 5.6% of the whole uncompressed theme size), but can have a positive impact in countries with slow internet connections.

The svg file was compressed by hand. The png files with pngquant.

/lib/Utils/TwigExtensionPlaceholderImage.php is also converting the image placeholder used for lazy-loading to a base64 data-uri, when 'data-uried' svg are known to be shorter in size and gzip better with proper encoding of symbols instead of base64 the whole image. You can have a look at this library for more info: https://github.com/tigt/mini-svg-data-uri/tree/master.

This is the only image that is shown in the front end (apart from Flynt's own website) and the gain is quite small. But it's so widely used (each image on every page on every site made with the theme) that might be worth digging into it.


@steffenbew
Copy link
Member

Hey @LuisSR, thanks a ton for your elaborate feedback! That's invaluable! Especially as it's so well documented and constructive. A couple of things are already on our list, some fixed already, others are new and some are conceptional thoughts we'll need to explain or rethink. Bear with us and give us some time to work through that! :)

@LuisSR
Copy link
Author

LuisSR commented Aug 18, 2023

Thank you, @steffenbew! I owe you all an apology for the length of the issue. 😆

I'll wait for any update, and I'll be glad to help you if I can.

It wasn't mentioned in the issue, but the theme is amazing!

@timohubois
Copy link
Contributor

@LuisSR many thanks for the feedback and deep dive into Flynt.

I will shortly try to provide an update:

1. Back end issues

1.1. The theme can be activated in servers with a PHP version lower than 7.4 and leave the WP installation unusable

The style.css file has been modified to include a “Requires PHP” line in its header.

1.2. Add component button is not working as expected

The PR is merged and currently we have no known problems with it. If it does not work for you or you figure other issues, let us know or create a new PR with your changes, please.

1.3. The custom post type boilerplate is showing the ACF components interface, but components are not working on the frontend

We have updated the “pageComponents.php” file in relation to the /inc/customPostTypes/example.php, we oriented the code to the output from the GenerateWP Post Type Generator and just want to provide a starting point and we will discuss in the team about adding show_in_rest to this file.

1.4. Some printed variables via Twig need escaping

We are aware of this as well and would really appreciate your ideas and improvements in a new PR.

2. Front end issues

2.1. Support for Safari v14 and lower is not complete

Flynt is following a boilerplate approach and has decided not to make any changes in this area at this time. In the Discussions section, you may also be interested in a thread on legacy support: #484

2.2. Posts created with the core/classic block or with the TinyMCE editor do not have their content wrapped

That's a really good point, and I hadn't noticed that before. We will open an internal issue to find a solution. Feel free to post a PR as well.

2.3. Small styling issues around the post header and meta

2.3.1. An extra divider is shown in the post meta when a post has no categories

This should be fixed in current master branch.

2.3.2. Author box and categories/reading time are not correcty aligned

This should be fixed in current master branch.

2.3.3. Bleeding is missing when a post has no featured image

This should be fixed in current master branch.

2.4. Refine the aria-label on components that show a grid of posts

This should be fixed in current master branch.

3. Suggestions

3.1. The default block editor options might be overwhelming for content editors

We decided to keep the current behaviour and give the developer the freedom to do what they want without providing a specific path.

3.2. Group CSS media queries

Really interesting topic and approach! We currently feel it should be a developer decision to use this approach or not, so that we decided to not add this feature to the core – at the moment.

3.3. Included assets can be further compressed

We have further compressed the screenshots and updated them to the current master branch. Regarding the logo and the placeholder image, we would be really happy to see you hand optimized svg inside and further adjustments a new PR.

Final words

We were really happy to get such kind of a detailed issue and looking forward to your feedback in the future and further suggestions and maybe PRs 🚀

@timohubois
Copy link
Contributor

Closed due inactivity. @LuisSR feel free to create a new issue, provide us more feedback or create a pr at any time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants