Get WordPress

Make WordPress Core

Opened 14 months ago

Closed 6 weeks ago

Last modified 6 weeks ago

#58475 closed defect (bug) (fixed)

Twenty Twenty-Three: Aubergine variation's background implementation causes any block that impacts page height to impact header

Reported by: annezazu's profile annezazu Owned by: karmatosed's profile karmatosed
Milestone: 6.6 Priority: low
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch needs-testing needs-refresh commit
Focuses: css Cc:

Description (last modified by sabernhardt)

Originally reported here and moving over: ​GB51248

This variation does the following for the background:

linear-gradient(180deg, var(--wp--preset--color--base) 0 min(24rem, 10%), var(--wp--preset--color--secondary) 0% 30%, var(--wp--preset--color--base) 100%)

As Justin notes, in particular, the min(24rem, 10%) is essentially saying to use the lower value (either 24rem or 10%). On pages with little content, it is pretty much guaranteed to be 10% of the <body> height.

As a result, any block that can change the page height will run into issues.

Change History (12)

#1 @sabernhardt
14 months ago

  • Description modified (diff)
  • Focuses css added
  • Summary changed from TT3 Aubergine variation: background implementation causes any block that impacts page height to impact header to Twenty Twenty-Three: Aubergine variation's background implementation causes any block that impacts page height to impact header

#2 @sabernhardt
14 months ago

  • Description modified (diff)

#3 @sabernhardt
14 months ago

Was the measurement intended as 10vh instead of 10%?

This ticket was mentioned in ​PR #4650 on ​WordPress/wordpress-develop by ​@mikachan.
14 months ago #4

  • Keywords has-patch added; needs-patch removed

This PR changes the height of the top of the background on the Aubergine variation, to hopefully improve its relative height to the page height.

I've swapped a min() calculation with clamp(), to try and make the height differences more gradual and make the height slightly larger when the page content is quite short.

Hopefully addresses concerns raised in ​https://github.com/WordPress/gutenberg/issues/51248. It would be great if @anarieldesign has a chance to take a look at these changes too.

Trac ticket: https://core.trac.wordpress.org/ticket/58475

Example of changes on a page with a short height:

Before After
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/1645628/7f097e2d-bea1-49f9-8ca9-997df88a1704 https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/1645628/e5414ffd-1aed-4ec9-a6fe-cb700810a644

#5 @mikachan
14 months ago

I've tried to improve this by using a clamp() calculation for the height rather than using min().

Was the measurement intended as 10vh instead of 10%?

The vh units are working better - I've swapped the previous 10vh for a min of 10vh and a max of 14vh. I think this is working better, but it would be great to hear what others think.

#6 @karmatosed
2 months ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to Future Release

#7 @mikachan
7 weeks ago

  • Owner set to mikachan
  • Status changed from new to assigned

#8 @karmatosed
6 weeks ago

  • Keywords needs-refresh added

@mikachan I wanted to check in on this, how are you feeling about it now and is it ready for a little refresh to get in? I am wondering about the refresh as see errors on the patch right now due to time.

#9 @mikachan
6 weeks ago

Thanks for the ping, @karmatosed! I think this is still good to bring in, as long as folks agree with the visual differences described in the screenshots. I don't think any errors on the patch will be related to the fix as it's just an update to the clamp values. I've updated the patch with the latest from trunk, hopefully this fixes the errors.

#10 @karmatosed
6 weeks ago

  • Keywords commit added
  • Milestone changed from Future Release to 6.6
  • Owner changed from mikachan to karmatosed

Thanks @mikachan that works for me. I think it's good to go then. I'll assign to myself and look to carry through to commit.

#11 @karmatosed
6 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 58467:

Twenty Twenty-Three: Resolves aubergine variation background implementation height impact on header.

The aubergine header causes issues when a block can change the page height. This resolves that swapping to clamp.

Props annezazu, sabernhardt, mikachan.
Fixes #58475.

#12 @mikachan
6 weeks ago

Thank you!

Note: See TracTickets for help on using tickets.