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

support for Gutenberg : cover image and other blocks #702

Closed
ghost opened this issue Oct 12, 2018 · 25 comments · Fixed by #723
Closed

support for Gutenberg : cover image and other blocks #702

ghost opened this issue Oct 12, 2018 · 25 comments · Fixed by #723

Comments

@ghost
Copy link

ghost commented Oct 12, 2018

@ghost ghost added the Needs specifications label Oct 12, 2018
@ghost ghost assigned ghost and eri-trabiccolo Oct 12, 2018
@ghost ghost added the enhancement label Oct 12, 2018
@ghost
Copy link
Author

ghost commented Oct 15, 2018

Cover Image

div#page .content .page-title, .content .page-image, .content .entry * {
    margin: 1.5em auto;
    max-width: 900px;
}

cover image can be used :

  • with any user custom container width
  • only when no sidebars

The overriden style will be applied only when a cover image is detected in the content <= how ?
=> when filtering the body_class

Enlargement

@eri-trabiccolo
Copy link
Collaborator

eri-trabiccolo commented Oct 18, 2018

About the cover image:
I'm playing just a little bit with the "suggested approach" above and I cannot see it as a good candidate... especially for Hueman.

Basically we have to climb too much in the html tree to remove the "contained width" hence add the max-width to too many elements, not only the article content elements but also author bio, related posts, comments etc, that will really create a big mess to me.

Also we have to force these elements box-sizing to be border-box, as we don't want any horizontal padding make their width grow.

Additionally: floating elements inside the article content (e.g. common alignright/alignleft) will go to the "full-right/left" of the page, how do we control them?

The thing is that themes like Hueman are by design not compatible with the gb cover image, we should change the html.

I would rather go with my original "soft CSS + JS" approach:

  1. the alignfull would be achieved this way, stretch the alignfull element to 100vw and center with the well-known translate+left technique, make sure the "page wrapper" doesn't allow horizontal overflowing -> otherwise horizontal scrollbar will appear.

In most of the cases this would be already enough as you will lose at most around 15px (browser scrollbar common width) on the right and on the left of the "cover" image background, which is totally fine, since it's a background cover image (hence you'll already lose part of the image on the top/bottom by design).

Anyway... js will come handy:

  1. with js set the alignfull block image width equal to the body (page-wrapper) width, this can be done by dynamically adding inline CSS do the document, rather than to the various elements. Of course this should be refreshed on window resize (debounced).

@ghost
Copy link
Author

ghost commented Oct 18, 2018

Let's go with your solution.
We also need to of course add_theme_support for align wide in the themes.

@eri-trabiccolo
Copy link
Collaborator

eri-trabiccolo commented Oct 19, 2018

Have not found yet a good way, using the guten-api, to understand if a specific block is in the content.
So at the moment the CSS rules apply even when no align(wide|full) blocks are present.
This should not be a real problem as only one CSS rule might be problematic (1):
402fbc7#diff-db064809acf51a8a37fc758961a32d83R282

I say "might" because I don't know why do we have this rule:
https://github.com/presscustomizr/hueman/blob/dev/assets/front/css/_parts/0_5_single_post_page.css#L14

Consideration over alignwide and alignfull:

  1. The alignfull option will produce a cover image full-window-width only if the page layout is not boxed, we only have one central column == no sidebars:

alignfull

  1. The alignwide optionn will produce a cover image which will extend from the left edge to right edge of the "article content", which means that it will "eat" the article container padding of 30px + 30px (the red arrows point to the padding the alignwide coverimage "eats"):

alignwide

  1. The alignfull option will then act as the alignwide when conditions in 1) are not met.

@ghost
Copy link
Author

ghost commented Oct 23, 2018

Works fine thanks.

Bu The fixed background option breaks your solution on my tests. Can you reproduce and push a fix if yes ?

2018-10-23_14-12-38

Results when fixed background is enabled

fixed_bg

@eri-trabiccolo
Copy link
Collaborator

let me see, I'm on it

@eri-trabiccolo
Copy link
Collaborator

Ah great, no way.. Looks like an issue with chrome when using the transform translate, while firefox just "suppresses" the fixed background position.

@ghost
Copy link
Author

ghost commented Oct 23, 2018

ah not cool.
Let's discuss it during the next dev chat then

@eri-trabiccolo
Copy link
Collaborator

Totally not cool :@

@eri-trabiccolo
Copy link
Collaborator

What do you think about:
#706
+
https://github.com/presscustomizr/hueman-front-js-parts/pull/4

You know I always would like to avoid the JS styling when there are CSS/html alternatives... but in this case I don't see them...

@ghost
Copy link
Author

ghost commented Oct 23, 2018

I'm on a Nimble release right now, but I'll check it in a moment.
Can you confirm that your new PR is passing the Gutenberg option(s) tests ( absolute , others ? ) successfully on chrome and firefox ?

@eri-trabiccolo
Copy link
Collaborator

yes tests passed on my side, alignfull: fixed and non
alignwide
other alignments ar enot impacted by this and the previous PR.

@ghost
Copy link
Author

ghost commented Oct 24, 2018

Works for me too. Well done.
Can you implement a similar approach with Customizr ?

@eri-trabiccolo
Copy link
Collaborator

Yes, will then push a separate PR for other blocks if needed, so that we can better exclude/include only the changes we want.

ghost pushed a commit that referenced this issue Oct 25, 2018
…gutenberg#10659, but posts created with the former cover-image block will still use the wp-block-cover-image css class

related to #702
@ghost
Copy link
Author

ghost commented Oct 25, 2018

Hi @eri-trabiccolo , I don't know if you are aware of this change in the latest version of Gutenberg...
the cover image block has been changed into a more generic cover block. I noticed it because I could not make the full width cover image work after updating Gutenberg... That's soooo cool from the Gutenberg team to change things like this a few days before releasing WP 5.0... :( I don'"t thinkg they care much about the rest of the world trying to adapt themes and plugins... anyway, we have no choice but follow this madness.
Can you adapt your cover-image PR for Customizr ( that I have not merged yet ), and add the wp-block-cover css class, just like I did for Hueman ?
(the former css class wp-block-cover-image must be kept for retrocompatibility )
Thanks !

@ghost
Copy link
Author

ghost commented Oct 25, 2018

@eri-trabiccolo forget my request about the Customizr PR's, I'll do it !
Have a good day

@eri-trabiccolo
Copy link
Collaborator

I can if you want, let me know.

@ghost
Copy link
Author

ghost commented Oct 25, 2018

I'm on it. thanks

@eri-trabiccolo
Copy link
Collaborator

Ok

@eri-trabiccolo
Copy link
Collaborator

But this is absolutely out of head.
Basically if you don't update the post you'll still have the old CSS class in your markup (as it is not dynamically generated in front, as a shortcode would do), this means that themes potentially should keep outdated CSS rules for decades.

@ghost
Copy link
Author

ghost commented Oct 25, 2018

yes.

@ghost ghost added the fixed label Oct 25, 2018
@ghost ghost closed this as completed Oct 25, 2018
@eri-trabiccolo
Copy link
Collaborator

eri-trabiccolo commented Nov 15, 2018

following:
https://wordpress.org/support/topic/full-width-block-support-gutenberg/#post-10883329

we seem to have treated only the cover image block (I actually don't think when I did this the image block could be wide/full-width ... but I might be wrong),
there are several other blocks that can be wide/full-width, and if we declare to support the align-wide property it'll be available for all those blocks, which means we have to style them.
(another arguable thing of gb of course, since it should be totally possible for a theme to support the wide/full alignment for some blocks but not for all)

@eri-trabiccolo
Copy link
Collaborator

see list of succesfully tested blocks with:
#723
https://github.com/presscustomizr/hueman-front-js-parts/pull/5

at:
#1601 (comment)

@eri-trabiccolo
Copy link
Collaborator

I've also found that a pretty long gutenberg post gets cut-off in the featured posts slider when displaying the full content...

Not sure why, needs investigation.

@ghost
Copy link
Author

ghost commented Nov 16, 2018

ok...weird !
Thanks for taking a look

@ghost ghost closed this as completed in #723 Nov 23, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant