Improve media & text block on IE. #10812

jorgefilipecosta · 2018-10-19T19:00:06Z

Description

Fixes: #11577
Grid layout on ie has some special needs.
This PR applies a series of changes to get to a point where the main block problems on IE are mostly solved.
The autoprefixer configuration was updated to support grid prefixes.

The remaining problem on IE is that on the frontend the media & content always appear at 50% width. The way to solve this problem would require that we set an IE-specific inline style on the markup of the block. So we would be transferring more bytes over the wire for all the users to fix the problem in this browser, so I opted to leave this change out. In my option, the resizing can be seen as a progressive enhancement.

Screenshots:

Before:

After:

youknowriad · 2018-10-27T09:29:17Z

packages/block-library/src/media-text/editor.scss

-	grid-area: resizer;
+	-ms-grid-column: 1;
+	-ms-grid-column-span: 2;
+	-ms-grid-row: 2;


Instead of adding this mse specific styles here, can we just enable grid: true in autoprefixer config?

Hi @youknowriad, I agree using the autoprefixer is an awesome idea and makes the code simpler.
I did some tests, and the template areas we had were not working even with the autoprefixer, setting columns spans using grid-column: 1 / -1; also did not work but using grid-column: 1 / span 2; works.
It looks like with this changes to the CSS and using the autoprefixer things are working fine in more modern browsers and in IE11, without having IE prefixes around the code.
Thank you for the suggestion.

I don't see the change to the autoprefixer config in the changes? Is it already enabled?

Sorry, I missed a stash on the change, the problem was corrected.

gziolo · 2019-02-07T16:10:08Z

@jorgefilipecosta this branch has merge conflicts and needs to be refreshed. It seems like it never to reviewed. Can you bring it up to date and remove Stale label which I'm applying to make review and triage process easier?

jorgefilipecosta · 2019-02-14T14:23:36Z

Hi @gziolo this PR was rebased and updated to work with the new "Media & Text" functionalities like the stack on mobile.

aduth · 2019-04-24T19:14:56Z

packages/block-library/src/media-text/edit.js

 		const style = {
-			gridTemplateColumns: 'right' === mediaPosition ? `auto ${ widthString }` : `${ widthString } auto`,
+			gridTemplateColumns,
+			msGridColumns: gridTemplateColumns,


Note that style attributes must be whitelisted. This was missed for grid-template-columns previously, and can result in invalidated blocks for non-administrator users.

See also: https://core.trac.wordpress.org/ticket/46597

We should probably add a filter to include -ms-grid-columns for the safe_style_css set in the plugin, and propose a complementing upstream Trac ticket once this is merged.

https://developer.wordpress.org/reference/hooks/safe_style_css/

Hi @aduth, this PR is not changing the block save function/markup of the block. At the time (before 5.0 release) in talks with @mtias it was discussed if we should add an IE specific style to the block inline styles and we decided not to do it. The disadvantage is that on the website frontend the image always appears at 50% width, on the backend we add the correct style so it appears on the correct width.

Given that we are not adding -ms-grid-columns on the save function I don't think there is a need to update filters compared to what we are already doing on the patches available at https://core.trac.wordpress.org/ticket/46597.

this PR is not changing the block save function/markup of the block

Ah, you're right. Sorry, I missed that.

I guess it makes me wonder then what's the purported benefit, if we're only solving the issue in the editor, but not on the front-end. To me, it would seem the latter would be the more important anyways. At the very least, should it not be marked as fixing #11577 ?

Currently, the block does not work in IE (front-end and editor), the media element and the content don't event appear side by side. This PR fixes most of the problems on the front end (in the editor fixes all of them). The layout appears side by side the vertical-align options work, the background position change works, the only thing that does not work is the resizing on the front end (e.g: things appear always with the default 50% width), compared to the current state is a big improvement. Issue #11577 says "Media & Text block is not aligning in IE 11" and I guess that problem is fixed the alignments work as expected, maybe we can close and add a comment specifying that just the resizing functionality does not work on the front end.

tyrann0us · 2019-05-10T10:13:55Z

@jorgefilipecosta, what's the status here? Thanks!
CC @aduth, @gziolo

jorgefilipecosta · 2019-05-13T12:17:05Z

Hi @tyrann0us, this rebased and updated. It is waiting for a review in order to be merged.

jorgefilipecosta · 2019-10-09T16:00:50Z

This PR was updated and should be ready just needs a review.

youknowriad

Seems to work well in my testing.

Squashed commits: [48c530fcb] Improve media&text block on IE.

Comments were addressed

Squashed commits: [48c530fcb] Improve media&text block on IE.

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from 26b99d2 to dd6f103 Compare October 26, 2018 15:23

jorgefilipecosta changed the title ~~WIP Improve media & text block on IE.~~ Improve media & text block on IE. Oct 26, 2018

jorgefilipecosta requested a review from a team October 26, 2018 20:45

youknowriad reviewed Oct 27, 2018

View reviewed changes

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch 4 times, most recently from dc9dfa8 to 66ed56e Compare October 29, 2018 15:42

Soean added the [Block] Media & Text Affects the Media & Text Block label Oct 31, 2018

gziolo added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Feb 7, 2019

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from 66ed56e to 7c7b6c6 Compare February 14, 2019 14:20

jorgefilipecosta requested review from aduth, ajitbohra, gziolo, nerrad, noisysocks, notnownikki, ntwb, Soean and talldan as code owners February 14, 2019 14:20

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from 7c7b6c6 to 1d26630 Compare February 14, 2019 14:21

jorgefilipecosta added Browser Issues Issues or PRs that are related to browser specific problems and removed [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. labels Feb 14, 2019

gziolo added this to the 5.2 (Gutenberg) milestone Feb 14, 2019

gziolo modified the milestones: 5.2 (Gutenberg), 5.3 (Gutenberg) Mar 4, 2019

jorgefilipecosta modified the milestones: 5.3 (Gutenberg), 5.4 (Gutenberg) Mar 14, 2019

mapk mentioned this pull request Apr 2, 2019

Media & Text Block: Add image fill option #14445

Merged

12 tasks

noisysocks modified the milestones: 5.5 (Gutenberg), 5.6 (Gutenberg) Apr 17, 2019

aduth previously requested changes Apr 24, 2019

View reviewed changes

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from 1d26630 to 0ef748a Compare April 29, 2019 14:39

gziolo removed this from the 5.6 (Gutenberg) milestone May 6, 2019

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from 0ef748a to 6c7efc1 Compare May 13, 2019 12:12

jorgefilipecosta requested a review from oandregal as a code owner May 13, 2019 12:12

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from 6c7efc1 to 566d358 Compare May 28, 2019 08:17

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from 566d358 to dc87785 Compare October 9, 2019 15:48

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from dc87785 to 61b1925 Compare October 10, 2019 15:38

youknowriad approved these changes Oct 11, 2019

View reviewed changes

post rebase fixes (+1 squashed commit)

4c7a142

Squashed commits: [48c530fcb] Improve media&text block on IE.

jorgefilipecosta force-pushed the fix/media-text-block-on-ie branch from 61b1925 to 4c7a142 Compare October 11, 2019 11:49

jorgefilipecosta merged commit 94057a3 into master Oct 11, 2019

jorgefilipecosta deleted the fix/media-text-block-on-ie branch October 11, 2019 12:55

youknowriad added this to the Gutenberg 6.7 milestone Oct 14, 2019

gziolo pushed a commit that referenced this pull request Oct 15, 2019

Improve media & text block on IE. (#10812)

eac1835

Squashed commits: [48c530fcb] Improve media&text block on IE.

mmtr mentioned this pull request Oct 18, 2019

Media & Text Block formatting issue when stack on mobile is enabled. Automattic/wp-calypso#36876

Closed

iamtakashi mentioned this pull request Oct 18, 2019

Business themes: Media & Text block ordering issue Automattic/themes#1521

Closed

eyesofjeremy mentioned this pull request May 19, 2020

Media & Text: Always show image first when "Stack on mobile" is toggled #14283

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve media & text block on IE. #10812

Improve media & text block on IE. #10812

jorgefilipecosta commented Oct 19, 2018 •

edited

Loading

youknowriad Oct 27, 2018

jorgefilipecosta Oct 29, 2018

youknowriad Oct 29, 2018

jorgefilipecosta Oct 29, 2018

gziolo commented Feb 7, 2019

jorgefilipecosta commented Feb 14, 2019

aduth Apr 24, 2019

jorgefilipecosta Apr 29, 2019

aduth Apr 29, 2019

jorgefilipecosta Apr 30, 2019

tyrann0us commented May 10, 2019

jorgefilipecosta commented May 13, 2019

jorgefilipecosta commented Oct 9, 2019

youknowriad left a comment

Improve media & text block on IE. #10812

Improve media & text block on IE. #10812

Conversation

jorgefilipecosta commented Oct 19, 2018 • edited Loading

Description

Screenshots:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented Feb 7, 2019

jorgefilipecosta commented Feb 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyrann0us commented May 10, 2019

jorgefilipecosta commented May 13, 2019

jorgefilipecosta commented Oct 9, 2019

youknowriad left a comment

Choose a reason for hiding this comment

jorgefilipecosta commented Oct 19, 2018 •

edited

Loading