New IE flexbox compat hint #4073

captainbrosset · 2020-10-08T16:05:49Z

This is an attempt to address #4072.

A hint that warns users when they do configure IE as a target browser and flexbox is used in one of the stylesheets.
This is mostly to get the ball rolling and gather feedback.

Let me know what you think.

Hint that warns users when they do configure IE as a target browser and flexbox is used in one of the stylesheets. This is the initial commit, with the boilerplate code only. More work is needed on the message, and on adding tests. This is just to get the ball rolling. Fix: webhintio#4072

We may also want to embed URLs for users to find out more, but this is a good first step.

sarvaje · 2020-10-08T17:28:20Z

packages/hint-ie-flexbox-compat/src/hint.ts

+ */
+
+export default class IEFlexboxCompatHint implements IHint {
+    public static readonly meta: HintMetadata = {


Meta needs to be in its own file (meta.ts) and imported. (e.g. https://github.com/webhintio/hint/blob/main/packages/hint-manifest-is-valid/src/hint.ts).
We use the meta file to generate the documentation in the website.

Thanks! Done.

antross · 2020-10-16T02:06:11Z

packages/hint-ie-flexbox-compat/src/_locales/en/messages.json

+    },
+    "usingFlexWithIE": {
+        "description": "String warning users that flex is used and IE is a target browser",
+        "message": "The Internet Explorer brower versions 11 and older suffer from a number of CSS Flexbox compatibility bugs. The layout on this page may be different between Internet Explorer and other, more recent, browsers. The size or alignment of elements may be incorrect and content may overflow.\nSupport for older versions of Internet Explorer ended and Microsoft no longer provides security updates or technical support for these versions."


Let's keep the message a bit more concise and leave the details for the README.

Suggested change

"message": "The Internet Explorer brower versions 11 and older suffer from a number of CSS Flexbox compatibility bugs. The layout on this page may be different between Internet Explorer and other, more recent, browsers. The size or alignment of elements may be incorrect and content may overflow.\nSupport for older versions of Internet Explorer ended and Microsoft no longer provides security updates or technical support for these versions."

"message": "CSS Flexbox may render differently in Internet Explorer than more recent browsers. Check this page in IE then see the documentation for workarounds if needed."

antross

Looks like a good start. We'll want a good README.md added as well (which will be published as the doc page on webhint.io). I'd expect to call out an example issue there then link to the full list of bugs + workarounds.

antross · 2020-10-16T02:10:23Z

packages/hint-ie-flexbox-compat/src/hint.ts

+
+        let reported = false;
+
+        context.on('parse::end::css', ({ ast, element, resource }: StyleParse) => {


Nit: I don't think you need to specify the type here as it should be inferred from the event name.

Suggested change

context.on('parse::end::css', ({ ast, element, resource }: StyleParse) => {

context.on('parse::end::css', ({ ast, element, resource }) => {

Sounds good, I will address this.

antross · 2020-10-16T02:12:02Z

packages/hint-ie-flexbox-compat/src/hint.ts

+            return;
+        }
+
+        let reported = false;


In the VS Code extension or CLI in watch mode this will never get reset between runs (causing the error message to disappear). Add a 'scan::end' listener to the context to reset this.

Thanks! Will do.

captainbrosset · 2020-10-16T15:11:55Z

Oh you're right, I forgot to add a README file. I'll simplify the message and instead write a longer README.

antross · 2020-10-30T22:17:20Z

/azp run

azure-pipelines · 2020-10-30T22:17:30Z

Azure Pipelines successfully started running 1 pipeline(s).

antross · 2020-11-02T17:27:30Z

@captainbrosset FYI just looks like this is hitting a linting failure on the updated README:


README.md: 3: MD013/line-length Line length [Expected: 80; Actual: 96]
README.md: 7: MD013/line-length Line length [Expected: 80; Actual: 87]
README.md: 8: MD013/line-length Line length [Expected: 80; Actual: 107]
README.md: 11: MD013/line-length Line length [Expected: 80; Actual: 109]
README.md: 15: MD013/line-length Line length [Expected: 80; Actual: 104]
README.md: 16: MD013/line-length Line length [Expected: 80; Actual: 105]
README.md: 20: MD013/line-length Line length [Expected: 80; Actual: 91]

antross

LGTM. Thanks @captainbrosset!

sarvaje

LGTM!

captainbrosset · 2020-11-05T08:01:55Z

As a follow-up, should we add this hint to the default configuration? And if so, am I right in assuming this is the place to do so: packages\configuration-web-recommended\index.json ?

antross · 2020-11-05T17:18:37Z

@captainbrosset that's one place, but there are also a few others to hit our default configurations in other contexts (browser extension, VS Code, etc.). There's probably a good TODO in here for establishing a common base config the others can extend from 😉

See this PR for a recent example: #4079

The online scanner also needs separately updated after a release. For now you can just log an issue like this one and @sarvaje will take care of it: webhintio/serverless-online-service#199

captainbrosset added 3 commits October 8, 2020 07:39

New: Tests for the new IE Flexbox Compat hint

cd12f7b

New: Make the IE Flexbox Compat message more final

4de1af7

We may also want to embed URLs for users to find out more, but this is a good first step.

captainbrosset requested review from antross, molant, sarvaje and utsavized as code owners October 8, 2020 16:05

sarvaje reviewed Oct 8, 2020

View reviewed changes

New: Making the ie flexbox compat meta a separate file

4dcc6f0

antross reviewed Oct 16, 2020

View reviewed changes

Fix: Addressing Tony's review comments

fda65fa

captainbrosset added 2 commits November 3, 2020 09:30

Fix: README.md formatting

48be1b1

Fix: Clean-ups

5d862d8

antross approved these changes Nov 3, 2020

View reviewed changes

sarvaje approved these changes Nov 3, 2020

View reviewed changes

captainbrosset merged commit a5a9753 into webhintio:main Nov 5, 2020

captainbrosset deleted the ie-flexbox-compat branch November 5, 2020 07:55

hxlnt mentioned this pull request Dec 9, 2020

Add rule that alerts if you are writing bogus code with flexbox #132

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New IE flexbox compat hint #4073

New IE flexbox compat hint #4073

captainbrosset commented Oct 8, 2020

sarvaje Oct 8, 2020

captainbrosset Oct 9, 2020

antross Oct 16, 2020 •

edited

Loading

antross left a comment

antross Oct 16, 2020

captainbrosset Oct 16, 2020

antross Oct 16, 2020

captainbrosset Oct 16, 2020

captainbrosset commented Oct 16, 2020

antross commented Oct 30, 2020

azure-pipelines bot commented Oct 30, 2020

antross commented Nov 2, 2020

antross left a comment

sarvaje left a comment

captainbrosset commented Nov 5, 2020

antross commented Nov 5, 2020


		let reported = false;

		context.on('parse::end::css', ({ ast, element, resource }: StyleParse) => {

New IE flexbox compat hint #4073

New IE flexbox compat hint #4073

Conversation

captainbrosset commented Oct 8, 2020

sarvaje Oct 8, 2020

Choose a reason for hiding this comment

captainbrosset Oct 9, 2020

Choose a reason for hiding this comment

antross Oct 16, 2020 • edited Loading

Choose a reason for hiding this comment

antross left a comment

Choose a reason for hiding this comment

antross Oct 16, 2020

Choose a reason for hiding this comment

captainbrosset Oct 16, 2020

Choose a reason for hiding this comment

antross Oct 16, 2020

Choose a reason for hiding this comment

captainbrosset Oct 16, 2020

Choose a reason for hiding this comment

captainbrosset commented Oct 16, 2020

antross commented Oct 30, 2020

azure-pipelines bot commented Oct 30, 2020

antross commented Nov 2, 2020

antross left a comment

Choose a reason for hiding this comment

sarvaje left a comment

Choose a reason for hiding this comment

captainbrosset commented Nov 5, 2020

antross commented Nov 5, 2020

antross Oct 16, 2020 •

edited

Loading