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

Basis & Flexible Layout Templates: Remove white space between two full width blocks placed next to each other. #4095

Open
stpaultim opened this issue Sep 26, 2019 · 15 comments · May be fixed by backdrop/backdrop#4959

Comments

@stpaultim
Copy link
Member

stpaultim commented Sep 26, 2019

Description of the bug
If while using a flexible layout template you put two blocks with background images or background colors that span the full width of screen, one after another, the Basis theme has a gap between them. I think it's a reasonable expectation that two hero type blocks one after another should butt up against each other.

(EDIT - Current PR expands scope a bit to include other blocks that might also stretch across full width of page. #4095 (comment))

Steps To Reproduce
To reproduce the behavior:

  1. Go to https://backdropcms.org/demo/create and spin up a demo site
  2. Go to /admin/structure/layouts/settings and create a flexible layout template
  3. Add a "hero" row and in Configure Row > Style > Row Width Bahavior choose "Always Full With of Page.
  4. Go to /admin/structure/layouts/manage/home/configure and select your flexible layout template for the home page.
  5. Manually place your blocks into the appropriate rows, make sure you have two hero blocks in your Hero row.
  6. Save settings and view home page

Actual behavior

You will see this gap between the two hero rows.

Home___Flexible

Expected behavior
I expect the hero rows to butt up against each other.

Additional information
How do we fix this without breaking existing sites that may have already made changes to fix this.

The file core/themes/basis/css/component/hero.css has the following class, which is causing this problem.

.block-hero {
margin: 0 0 2rem;
}

In Tatsu I solved this problem by overriding this class with "margin-bottom: 0;"

Can we just remove this margin? Does this not risk breaking existing sites that might have already themed around this problem or have sites for which the gap is now expected?

@jenlampton
Copy link
Member

Can we just remove this margin?

I'd say yes. But we need to add a margin back to something on the Basis home page. We can specifically target the basis home page with body.page-front or even .l-boxton. Let's find an element where its safe to add the margin :)

@ghost
Copy link

ghost commented Aug 10, 2020

I disagree that this is an issue we need to fix. This is making a lot of assumptions about how regions/blocks should be displayed, and how people want their site to look.

For example, if you don't create a hero-specific region, and just add two hero blocks to the top of the Content region, then the bottom margin nicely adds whitespace between the hero block and the content:
image

I don't think we can assume that hero blocks should never have a bottom margin, especially since people can put them anywhere, with anything immediately below them.

As for @jenlampton's suggestion about adding the margin elsewhere, I don't see how that'd be possible without making assumptions about how people use blocks and/or requiring them to use blocks in a specific way.

This issue isn't present in Backdrop OOTB, only if you use a custom, flexible template. And flexible templates are, by definition, flexible. So I'd say close this issue and leave it up to individual sites to style their blocks as appropriate.

@olafgrabienski
Copy link

This is making a lot of assumptions about how regions/blocks should be displayed, and how people want their site to look.

The assumptions are: putting two (or more) hero blocks one after another in a flexible layout template. While I'm also not sure if we need to fix the issue, it would be nice, and maybe it's quite easy: leave the margin-bottom as is and add a negative margin-top to every hero block which directly follows another one.

@ghost
Copy link

ghost commented Aug 10, 2020

I'd clarify the assumptions as:

  1. Assuming people add more than one hero block to the same region
  2. Assuming they use a flexible layout template
  3. Assuming nothing follows the hero blocks (or whatever does should butt up against them)
  4. Assuming people don't actually want a gap between the hero blocks

@quicksketch
Copy link
Member

Could we perhaps solve this problem as a very narrow situation of "Assuming people don't actually want a gap between the hero blocks"? We could potentially solve exactly only this situation with a "sibling" CSS selector:

.block-hero {
  margin: 0 0 2rem;
}
.block-hero + .block-hero:not(:last) {
  margin: 0;
}

Not 100% sure on the syntax there but I think it's achievable.

@jenlampton
Copy link
Member

jenlampton commented Nov 5, 2020

Assuming they use a flexible layout template

@BWPanda This assumption isn't necessary, I've had hero blocks get the unwanted margin-bottom even using core and custom layouts.

Could we perhaps solve this problem as...

@quicksketch there are lots of possible ways to prevent the margin-bottom from appearing, but all of them will affect existing sites using Basis. We really need to reach a consensus on #4167 before we can move forward with any breaking CSS changes in 1.x.

@olafgrabienski
Copy link

New idea: What about adding a setting for hero blocks with the option to display it with or without margin-bottom? The setting could trigger the addition of one or several CSS helper classes, see e.g. https://getbootstrap.com/docs/4.5/utilities/spacing.

@stpaultim
Copy link
Member Author

stpaultim commented Nov 30, 2020

New idea: What about adding a setting for hero blocks with the option to display it with or without margin-bottom?

This is interesting. It would allow us to deal with the backward compatibility situation. My first reaction was positive, but this would mean one more configuration in the UI and I'm not sure it's worth it. In my view, if we come up with some way to make sustainable changes to Basis then the config is not needed. If we don't, then I would recommend people use contrib themes instead anyway and encourage contrib theme developers to fix it.

The goal was to make Basis more usable out of the box, but if that's not going to be possible - then I agree that there is no need to fix this specific problem out of the box.

I'm also come to realize that basis does already have a partial fix for this core default. If you put 2 hero blocks in the top region (.l-top) in Basis, it already makes the adjustment (it adds a rule margin-bottom: -2rem; to .l-top .block-hero). This is why we mostly see this problem in flexible templates. So Basis already assumes that there should NOT be a margin between two hero blocks, but ONLY if they are in a region with the class l-top.

A possible work-a-round is for the site builder to give the region the class .l-top and then this problem does not happen (assuming the blocks have the class = .block-hero). I've been having this problem with blocks that I convert to become hero blocks using Configurable Block Styles. They don't currently have the .block-hero class. But, we could make sure that Configurable Block Styles automatically adds this class for hero blocks.

@izmeez
Copy link

izmeez commented Feb 22, 2024

A previous comment in this thread about possibly using "sibling" CSS selectors looks interesting. This is both new to me and intriguing, so I'll explore it further.

@stpaultim
Copy link
Member Author

stpaultim commented Dec 29, 2024

I'm experimenting with a PR for this issue. Some assumptions I'm making:

  1. This PR will effect all blocks
  2. I'm testing it with and without Configurable Block Styles module.

This PR currently has a few CSS rules that do the following:

  1. Remove the 2rem margin at the bottom of hero blocks.
.update-1-30 .block-hero {
  margin: 0;
}
  1. Make sure there is some padding at bottom of region, if region has container. This is important if region has background color (see header region).
.update-1-30 .container .l-flexible-row {
  padding-bottom: 2rem;
}
  1. Remove margin below header.
.update-1-30 .l-header {
  margin-bottom: 0;
}

I have tested this with the following:

  1. Boxton Layout and Flexible Layout Template (3 Regions)
  2. The following blocks
  • Header block
  • Primary Navigation
  • Recent Content View
  • 3 Cards
  • Promoted Content
  • Two Hero blocks (default (Welcome block) + 1 hero block with image)
  • Custom block, using configurable block styles module. Gave block a background color.

I've taken these blocks and dragged them around into many orders and placements trying to find weird problems that did not already exist in basis.

I've tested the Flexible Layout Template with regions that have no container and with regions that do have container.

@olafgrabienski
Copy link

@stpaultim As your PR affects all blocks which can have side effects, have you considered to use the approach mentioned by @quicksketch in #4095 (comment) instead? That one affects only hero blocks.

@stpaultim
Copy link
Member Author

stpaultim commented Dec 29, 2024

@olafgrabienski Yes, I considered that. I would very much prefer that we expand the scope slightly to include other blocks, because the use case for two hero blocks is very limited. I believe that this same problem exists for other blocks and is a situation that is more common. The problems that are addressed with this PR, because much more visible with flexible layouts. Until now, we have not been able to fix problems in Basis that effect flexible layouts.

I understand that this does potentially increase the risk of unintended side effects, which is why I've tried to test this with lots of different blocks. The risk is also mitigated, because this will only effect new sites and in my opinion, the status quo is quite bad, so the risk of making things worse is very low.

I will try to record a demo of the kinds of situations that concern me here, because I think it's hard to describe without a demo.

I will say, that this very small PR, is in my opinion a huge improvement for Basis (pending more testing).

@stpaultim
Copy link
Member Author

stpaultim commented Dec 30, 2024

In each image the following three blocks are: Hero Block, Hero Block, and Custom Block (using configurable block styles).

CURRENT CODE - Default Layout
image

CURRENT CODE - Flexible Layout Template (Header Region is Full Width - hero blocks in header region)
image

NEW PR - Default Layout - INCLUDING NEW PR
image

NEW PR - Flexible Layout Template (Header Region is Full Width - hero blocks in header region)
image

@stpaultim stpaultim changed the title Basis & Flexible Layout Templates: Remove white space between two hero blocks placed next to each other. Basis & Flexible Layout Templates: Remove white space between two full width blocks placed next to each other. Jan 3, 2025
@stpaultim
Copy link
Member Author

I've edited the title and original description of this issue to make clear that this issue should not be limited to "hero blocks" in my opinion.

@stpaultim
Copy link
Member Author

stpaultim commented Jan 3, 2025

I had a short discussion with @jenlampton about what I'm trying to fix in Basis and am considering that my fix might not be the right approach for this problem.

My goal is make it possible with Basis to place two full width blocks, with either background images or background colors, next to each other in a region without a container class (full-width). I'd like to do this without any custom code or CSS.

Currently, there are only two places to do this in core. Either in the top region, when using Boxton on the front page OR when using flexible layouts in regions without a container class.

Since there is no way to create a block with a different color background in core, this situation is really only possible with contrib modules such as "Configurable Block Styles".

While the inability to create the layout I described is a problem, it might be that a better fix in core would be for a new block type. We could then make sure that this new block type is properly themed in Basis. This problem could be fixed in Basis without a new block type, but it may be that the solution is a bit too hacky.

I need to think about this some more and experiment a bit. This will require more thought and may be a discussion topic at next weeks Design/UX meeting.

In short, I think I'm putting this PR on hold until our next meeting and getting some feedback at that time. We still have two weeks until the next release.

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.

6 participants