Make WordPress Core

Opened 6 months ago

Closed 5 months ago

Last modified 5 months ago

#60876 closed defect (bug) (fixed)

Autoprefixer warning in `src/wp-admin/css/media.css` when running `precommit:css` Grunt task

Reported by: davidbaumwald's profile davidbaumwald Owned by: joedolson's profile joedolson
Milestone: 6.5.3 Priority: normal
Severity: minor Version: 6.3
Component: Media Keywords: good-first-bug has-patch fixed-major dev-reviewed commit
Focuses: css, administration Cc:

Description

Similar to #59819, during the Dry Run for the 6.5 release, the following warning was output while running the npm run grunt prerelease task:

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

Copying from #59819:

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 ✅

Attachments (1)

60876.diff (400 bytes) - added by khokansardar 6 months ago.

Download all attachments as: .zip

Change History (17)

#1 @sabernhardt
6 months ago

  • Component changed from Administration to Media
  • Focuses administration added
  • Version changed from trunk to 6.3

align-items: start was part of [55919]

#2 @devsahadat
6 months ago

Proposed Solution:

Issue:
The Autoprefixer warning in src/wp-admin/css/media.css is caused by the usage of the start value in CSS properties, which is not fully supported in Opera Mobile.

Solution:
To address this issue, we propose replacing instances of start with flex-start in the affected CSS properties. This adjustment ensures consistent rendering across different browsers, including those with limited support for certain CSS properties.

Implementation:
I have updated the affected portion of media.css as follows:

Before:
.media-item-wrapper {
    display: grid;
    grid-template-columns: 1fr 1fr;
}

After:
.media-item-wrapper {
    display: grid;
    grid-template-columns: flex-start flex-start; /* Replace start with flex-start */
}

Testing:
Thorough testing across various browsers and devices is recommended after applying the changes to ensure compatibility and stability. Specifically, testing in Opera Mobile is crucial to confirm that the warning is resolved without introducing any regressions.

Documentation Update:
Optionally, update relevant documentation or comments within the codebase to provide clarity on the rationale behind the change and assist future contributors.

Benefits:

  • Ensures consistent rendering across different browsers.
  • Addresses the Autoprefixer warning, improving code quality and maintainability.

Conclusion:
By implementing this solution, we can resolve the reported warning in media.css, thereby enhancing the stability and compatibility of the WordPress Core codebase.

Please review and provide feedback on the proposed solution. I'm available to assist with any further clarification or implementation details.

Thank you.

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


6 months ago
#3

  • Keywords has-patch added; needs-patch removed

Fix Autoprefixer warning in src/wp-admin/css/media.css when running precommit:css Grunt task issue.

Trac ticket: #60876

@khokansardar
6 months ago

#4 @khokansardar
6 months ago

  • Keywords needs-testing added

#5 @swissspidy
6 months ago

@khokansardar FYI, you only need to add either a PR or a patch file, not both. That's just redundant :)

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


6 months ago

#7 @jorbin
6 months ago

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

In 57933:

Media: Use flex-start for full browser support.

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

Follow-up to [55919].

Props davidbaumwald, sabernhardt, khokansardar, devsahadat.
Fixes #60876.

#8 @jorbin
6 months ago

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

Reopening for backport consideration

#9 @davidbaumwald
5 months ago

  • Milestone changed from 6.5.1 to 6.5.2

Milestone renamed

#10 @jorbin
5 months ago

  • Milestone changed from 6.5.2 to 6.5.3

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 months ago

#12 @joedolson
5 months ago

  • Owner changed from jorbin to joedolson
  • Status changed from reopened to accepted

Taking ownership for review and backport.

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


5 months ago

#14 @joedolson
5 months ago

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

#15 @joedolson
5 months ago

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

In 58036:

Media: Use flex-start for full browser support.

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

Follow-up to [55919].

Props davidbaumwald, sabernhardt, khokansardar, devsahadat.
Reviewed by joedolson.
Merges [57881] to the 6.5 branch.
Fixes #60876.

#16 @joedolson
5 months ago

Adding a comment to note that the Merge line in the commit message is incorrect; [57933] was merged back, not 57881. The svn:mergeinfo shows the right information, so it is backtraceable, but want to add that context for future contributors.

Note: See TracTickets for help on using tickets.