Make WordPress Core

Opened 7 months ago

Closed 6 months ago

Last modified 2 months ago

#59819 closed defect (bug) (fixed)

Change CSS align-item from start / end to flex-start / flex-end for full browser support

Reported by: hellofromtonya's profile hellofromTonya Owned by: jorbin's profile jorbin
Milestone: 6.4.2 Priority: normal
Severity: normal Version: 6.4
Component: Administration Keywords: has-patch dev-reviewed commit
Focuses: css Cc:

Description

During today's Dry Run npm run grunt prerelease step, precommit:css grunt task flagged the following warnings for CSS align-items property values:

>> autoprefixer: F:\WordPress Core Develop\develop.svn.wordpress.org\branches\6.4\src\wp-admin\css\about.css:537:2: start value has mixed support, consider using flex-start instead
>> autoprefixer: F:\WordPress Core Develop\develop.svn.wordpress.org\branches\6.4\src\wp-admin\css\about.css:538:2: end value has 
mixed support, consider using flex-end instead
>> autoprefixer: F:\WordPress Core Develop\develop.svn.wordpress.org\branches\6.4\src\wp-admin\css\media.css:1127:2: start value has mixed support, consider using flex-start instead

The start and end values are not supported in Opera Mobile which has a usage of 1.01%.

On WordPress' Browsers Support page:

Last 2 Opera versions.

Browsers with > 1% usage based on can I use browser usage table.

[https://caniuse.com/usage-table Opera Mobile's last 2 versions have a usage of}:

  • 73: 1.01%
  • 12.1: 0%

Noting:
Flexbox is well supported on all browser versions that WordPress supports ✅

Change History (16)

#1 @NekoJonez
7 months ago

I tested it via WordPress playground on my Opera mobile, (I use Opera as my main and the sync on my phone is just nice) and didnt see any major or even minor jank... https://wordpress.slack.com/archives/C02RQBWTW/p1699295618024189?thread_ts=1699295618.024189&cid=C02RQBWTW

#2 @hellofromTonya
7 months ago

Reference: the full discussion in Make/Core slack from today's Dry Run results.

This ticket is slated for 6.4.1. Once it opens, then this ticket can be moved into that milestone.

Why not change it for 6.4.0?
This was discussed today but decided to punt to 6.4.1. Why?

Changing the alignment may have an unintended effect.

At this very very late stage in the release, consensus was to discuss first and then take the time to ensure the fix does not introduced an unintended effect.

This ticket was mentioned in PR #5627 on WordPress/wordpress-develop by @kebbet.


7 months ago
#3

  • Keywords has-patch added

https://core.trac.wordpress.org/ticket/59819

PR is addressing the warnings running npm run grunt prerelease.

  • Line 96 has align-self: start; but renders no warning.
  • Line 107 has align-self: end; but renders no warning.
  • Line 1137 has align-self: end; but renders no warning.

How come?

#4 @hellofromTonya
7 months ago

  • Milestone changed from Awaiting Review to 6.4.1

#5 @jorbin
7 months ago

  • Milestone changed from 6.4.1 to 6.4.2

#6 @NekoJonez
7 months ago

Apart from looking at the about page, is there another way to test how good this works or is this just something that can't be really tested?

#7 @jorbin
7 months ago

I think that visual testing in multiple browsers makes sense.

Also to be extra cautious, tagging @luminuu who wrote this originally to give you a chance to chime in on if there was a specific reason start and end were used.

#8 @luminuu
7 months ago

Thanks for the ping - I don't think there's any real reason for it other than "laziness" of skipping the flex- prefix and the fact that I was unaware align-items: start and align-items: end are not supported in Opera Mobile. Adding the flex- prefix does not change the behaviour in my tests in other browsers and if it helps for the linter and in Opera Mobile, I'm all fine merging the PR.

#9 @jorbin
6 months ago

  • Owner set to jorbin
  • Resolution set to fixed
  • Status changed from new to closed

In 57151:

Help/About: Change CSS align-item for full browser support.

The values of start and end are not fully supported by Opera Mini which has 1.01% usage. There is no material change in functionality with this change.

Props kebbet, NekoJonez, luminuu, hellofromTonya.
Fixes #59819.

#10 @jorbin
6 months ago

  • Keywords dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for consideration of [57151] to backport to 6.4.

#11 @hellofromTonya
6 months ago

  • Keywords dev-reviewed commit added; dev-feedback removed

[57151] LGTM for backporting to 6.4 :)

#12 @jorbin
6 months ago

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

In 57160:

Help/About: Change CSS align-item for full browser support.

The values of start and end are not fully supported by Opera Mini which has 1.01% usage. There is no material change in functionality with this change.

Reviewed by hellofromTonya.
Merges [57151] to 6.4 branch.

Props kebbet, NekoJonez, luminuu, hellofromTonya.
Fixes #59819.

@kebbet commented on PR #5627:


6 months ago
#13

Fixed in r57160.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


2 months ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


2 months ago

Note: See TracTickets for help on using tickets.