Giter Club home page Giter Club logo

Comments (15)

samikeijonen avatar samikeijonen commented on August 14, 2024 2

@colorful-tones Even that is not needed because there is wp-block-embed__wrapper class which we can target. Example markup:

<figure class="wp-block-embed-youtube alignwide wp-block-embed is-type-video is-provider-youtube wp-embed-aspect-16-9 wp-has-aspect-ratio">
   <div class="wp-block-embed__wrapper">
      <iframe width="500" height="281" src="https://www.youtube.com/embed/thIlxChNYqk?feature=oembed" frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" allowfullscreen="">
      </iframe>
   </div>
</figure>

Example CSS.

from theme-scaffold.

samikeijonen avatar samikeijonen commented on August 14, 2024 1

It's definitely hard balance what to include in theme scaffold. I personally dequeue all Gutenberg styles in the front-end, and roll my own. I have article coming on the subject, I'll try to link it here for reference.

from theme-scaffold.

colorful-tones avatar colorful-tones commented on August 14, 2024 1

@samikeijonen I do not know how I overlooked that simpler approach. Love it. I'm not entirely sold on the directory organization, but that example embeds.css partial should definitely make its way into Theme Scaffold.

from theme-scaffold.

colorful-tones avatar colorful-tones commented on August 14, 2024

Also, new hooks for enqueue and dequeue Gutenberg scripts: enqueue_block_editor_assets(), enqueue_block_assets() worth keeping on radar. Should we want to dequeue Gutenberg stuff and then have our own baseline.

from theme-scaffold.

colorful-tones avatar colorful-tones commented on August 14, 2024

my understanding is that core blocks have the potential following CSS files within them:

  • editor.scss <-- only affects the Gutenberg editor
  • style.scss <-- affects both the Gutenberg editor and frontend block
  • theme.scss <-- only has affect if add_theme_support( 'wp-block-styles' ); is set in theme

See Gutenberg Issue #6947 - Support adding opt-in visual styles for core blocks

from theme-scaffold.

colorful-tones avatar colorful-tones commented on August 14, 2024

add_theme_support( 'disable-custom-font-sizes' );

From the Gutenberg Handbook: "Disabling custom font sizes"

Themes can disable the ability to set custom font sizes with the following code: add_theme_support( 'disable-custom-font-sizes' );

When set, users will be restricted to the default sizes provided in Gutenberg or the sizes provided via the editor-font-sizes theme support setting.

With that understanding then the next logical step would be to leverage the add_theme_support( 'editor-font-sizes' ); hook. See next section on why this is not fully useable yet.

add_theme_support( 'editor-font-sizes' );

The immediate use-case that I thought would be most beneficial for add_theme_support( 'editor-font-sizes' ); would be to remove the ability for authors to change font sizes. This would involve the following in the theme's functions.php (or in 10up Theme Scaffold's case: includes/core.php):

// Pass empty array of font size so Gutenberg's core will not show.
add_theme_support( 'editor-font-sizes', array() );

Apparently this was on Jodi's (@JodiWarren) radar too, because he opened an Issue #11628 on Gutenberg in order to allow for this.

Of course, another approach for using the add_theme_support( 'editor-font-sizes' ); would be to add some default, and "sane" font-sizes in the Theme Scaffold, like:

add_theme_support( 'editor-font-sizes', array(
    array(
        'name' => __( 'Small-ish', 'tenup-scaffold' ),
        'size' => 12,
        'slug' => 'small_ish'
    ),
    array(
        'name' => __( 'Normal-ish', 'tenup-scaffold' ),
        'size' => 16,
        'slug' => 'normal_ish'
    ),
    array(
        'name' => __( 'Large-ish', 'tenup-scaffold' ),
        'size' => 36,
        'slug' => 'large_ish'
    ),
    array(
        'name' => __( 'Huge-ish', 'tenup-scaffold' ),
        'size' => 50,
        'slug' => 'huge_ish'
    )
) );

But, what those "sane" sizes might be would likely be hard to decide upon and potentially cause more harm than value in the Theme Scaffold. Unless, there was a Scaffolding Checklist where font sizes were updated when a new project is spun up.

This approach would also requires us to add some additional CSS to style what ever custom font sizes we registered, e.g.

.has-large_ish-font-size {
    font-size: 36px;
}

add_theme_support( 'disable-custom-colors' );

Same info as add_theme_support( 'disable-custom-font-sizes' ); (above) really.

add_theme_support( 'editor-color-palette' );

Same info as add_theme_support( 'editor-font-sizes' ); (above) really. We could register some basic, default colors in the Theme Scaffold, but what those colors are, and whether they offer value for jump-starting a new project seems to offer low value.

add_theme_support( 'responsive-embeds' );

This seems like it could have some useful implications in incorporating in to the Theme Scaffold. Unfortunately, I have not had enough time to try it and dig on the full implications. I.e. needs further digging.

Relevant links:

from theme-scaffold.

colorful-tones avatar colorful-tones commented on August 14, 2024

@samikeijonen I would be interested to read your post, because dequeuing everything would likely be my suggestion. It is a similar approach I use when doing WooCommerce sites.

from theme-scaffold.

colorful-tones avatar colorful-tones commented on August 14, 2024

add_theme_support( 'responsive-embeds' );

If this is added to a theme. It merely adds a body class .wp-embed-responsive and some additional CSS is also output. It uses the good old aspect ratio (aka EmbedResponsively.com) trick to try and make embeds (iframe, object, embed) responsive.

Personally, I'm not a fan of that approach. Mostly, because there is the occassional need for having an embed in a header (e.g. hero, full screen Youtube video) and then there might be some conflicts.

I prefer to use the add_filter( 'embed_oembed_html' ); approach and have something like this in the theme instead:

add_filter( 'embed_oembed_html', 'responsive_oembed_html', 10, 3 );

/**
 * Adds a responsive embed wrapper around oEmbed content
 * @param  string $html The oEmbed markup
 * @param  string $url  The URL being embedded
 * @param  array  $attr An array of attributes
 * @return string       Updated embed markup
 */
function responsive_oembed_html( $html, $url, $attr ) {
	$classes = array();

	// Add these classes to all embeds.
	$classes_all = array(
		'responsive-media',
	);

	// Check for different providers and add appropriate classes.
        // This part is not necessary, but nice to have the option sometimes...
	if ( false !== strpos( $url, 'vimeo.com' ) ) {
		$classes[] = 'vimeo';
	}

	if ( false !== strpos( $url, 'youtube.com' ) ) {
		$classes[] = 'youtube';
	}

	if ( false !== strpos( $url, 'wistia.net' ) ) {
		$classes[] = 'wistia';
	}

	$classes = array_merge( $classes, $classes_all );

	return '<div class="' . esc_attr( implode( $classes, ' ' ) ) . '">' . $html . '</div>';
}

from theme-scaffold.

colorful-tones avatar colorful-tones commented on August 14, 2024

I would vote that we refrain from adding most of the Gutenberg add_theme_support() stuff, and leave it to a project-by-project basis.

I do think we should add something like @samikeijonen 's Uuups theme's blocks directory to Theme Scaffold, but that is not related to this ticket.

from theme-scaffold.

samikeijonen avatar samikeijonen commented on August 14, 2024

It's definitely hard balance what to include in scaffold. Even more so because we don't know is block editor used or not.

Here are my thoughts based on assumption that block editor is common workflow in upcoming 10up projects.

What should go in

  • add_theme_support( 'disable-custom-colors' );
  • add_theme_support('disable-custom-font-sizes');

For most client sites I don't see us allowing any custom colors or font sizes which are added as inline styles.

What should not go in

add_theme_support( 'wp-block-styles' );

This outputs block-library/theme.css also in the front-end, it's already in the editor. But in my opinion we shouldn't use another Core stylesheet in the front-end. In fact we should dequeue Core block stylesheet and roll our own.

// Dequeue Core block styles.
wp_dequeue_style( 'wp-block-library' )

But that's more related to styles and issue #83. I'll comment separately in there.

add_theme_support( 'editor-styles' );

What happens to editor styles and how they are rolled can be decided on project by project. I personally use PostCSS to prefix all editor styles with .editor-styles-wrapper class if we need good match with editor and front-end.

add_theme_support( 'responsive-embeds' );

This is not needed as mentioned in the comment above.

add_theme_support( 'dark-editor-style' );

Needed only when styling dark editor experience.

Perhaps

add_theme_support( 'align-wide' );

If going with Gutenberg, it's hard to see why not adding support for alignwide and alignfull blocks. What these actually means for design, is up to the theme. I'd add pretty much empty classes.

I vote for adding align-wide.

  • add_theme_support( 'editor-color-palette' )
  • add_theme_support( 'editor-font-sizes' )

In most cases we would overwrite or remove Core color palette and font sizes. This is also related to adding example styles.

For colors I'd add only black and white since we already have them as example colors.

If I have to vote, I'd say no to this.

from theme-scaffold.

saltcod avatar saltcod commented on August 14, 2024

Thanks all! Agreed on all fronts except one it looks like.

My PR currently includes these:

add_theme_support( 'disable-custom-font-sizes' );

// If we disable custom colours, we need to provide a fallback, or the disabling doesn't work
add_theme_support( 'disable-custom-colors' );

// The fallback here includes just black and white
add_theme_support( 'editor-color-palette', array(
	array(
		'name' => 'black',
		'slug' => 'black',
		'color' => '#000',
	),
	array(
		'name' => 'white',
		'slug' => 'white',
		'color' => '#fff',
	),
) );

The one I'm on the fence on is add_theme_support( 'responsive-embeds' );.

I feel like this will prevent that super-common issue where embeds are getting cut off on smallest screens. But I agree that it may conflict with an embed being used somewhere else (like in a hero as Damon said).

I think the benefit outweighs the risk here: there's a bunch of possible embeds and it's easy to forget to check that these are responsive β€”Β and it's a quite common QA issue. If you're doing something like making an embed the background for a hero, you'll be writing custom styles for that anyway and can easily work around anything that responsive-embeds is adding.

tl; did read:
I think we're in agreement on

  • add_theme_support('disable-custom-font-sizes')
  • add_theme_support( 'disable-custom-colors' )
  • add_theme_support( 'editor-color-palette' )
  • add_theme_support( 'editor-font-sizes' )

But don't have agreement on:

  • add_theme_support( 'responsive-embeds' )

As always, open to suggestion.

from theme-scaffold.

samikeijonen avatar samikeijonen commented on August 14, 2024

@saltcod add_theme_support( 'responsive-embeds' ) adds wp-embed-responsive body class. I don't see that resolving any issues with iframes which can't be done in the embed block itself.

from theme-scaffold.

samikeijonen avatar samikeijonen commented on August 14, 2024

To @saltcod list I think add_theme_support('disable-custom-font-sizes') is missing.

from theme-scaffold.

colorful-tones avatar colorful-tones commented on August 14, 2024

@saltcod

Example 1: A typical oEmbed output with Gutenberg on front end:

<figure class="wp-block-embed-vimeo alignwide wp-block-embed is-type-video is-provider-vimeo wp-has-aspect-ratio wp-embed-aspect-16-9">
    <div class="wp-block-embed__wrapper">
        <iframe src="https://player.vimeo.com/video/22439234?app_id=122963" width="640" height="360" frameborder="0" title="The Mountain" allow="autoplay; fullscreen" allowfullscreen=""></iframe>
    </div>
</figure>

Example 2: With add_theme_support( 'responsive-embeds' ) added in the theme we get the addition of a body class:

<body class="wp-embed-responsive">

    ...

    <figure class="wp-block-embed-vimeo alignwide wp-block-embed is-type-video is-provider-vimeo wp-has-aspect-ratio wp-embed-aspect-16-9">
        <div class="wp-block-embed__wrapper">
            <iframe src="https://player.vimeo.com/video/22439234?app_id=122963" width="640" height="360" frameborder="0" title="The Mountain" allow="autoplay; fullscreen" allowfullscreen=""></iframe>
        </div>
    </figure>

Example 3: We don't use the add_theme_support( 'responsive-embeds' ), but instead use the embed_oembed_html filter, as I outlined above:

<figure class="wp-block-embed-vimeo alignwide wp-block-embed is-type-video is-provider-vimeo wp-has-aspect-ratio wp-embed-aspect-16-9">
    <div class="wp-block-embed__wrapper">
        <div class="vimeo responsive-media">
            <iframe src="https://player.vimeo.com/video/22439234?app_id=122963" width="640" height="360" frameborder="0" title="The Mountain" allow="autoplay; fullscreen" allowfullscreen=""></iframe>
        </div>
    </div>
</figure>

Note the additional wrapping <div class="vimeo responsive-media"> in this πŸ‘† last example.

I believe Example 1 is ideal, i.e. we do not add the add_theme_support( 'responsive-media' ). This would require that we add something like blocks/index.css, which imports blocks/embeds.css, and I would use what @samikeijonen has cleverly integrated in his Uuups: https://github.com/samikeijonen/uuups/blob/master/resources/css/blocks/core/embeds.css
This πŸ‘† addition would be more related and a PR for #83

from theme-scaffold.

timwright12 avatar timwright12 commented on August 14, 2024

closing since we implemented something similar to this

from theme-scaffold.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.