fix(curriculum): changed test to use regex

[ShaunSHamilton]

Checklist:

• I have read freeCodeCamp's contribution guidelines.
• My pull request has a descriptive title (not a vague title like Update index.md)
• My pull request targets the master branch of freeCodeCamp.
• All the files I changed are in the same world language, for example: only English changes, or only Chinese changes, etc.

Closes #38712

This change allows users to pass the test with correct code, when browser zoom is not at 100%.

NOTE: Code allows poorly formatted code to pass, whilst the image is still displayed correctly.

[gitpod-io]

[ShaunSHamilton]

I am struggling to get a robust regex that does allow this to pass:

.smaller-image {
  border-radius: 5px;
  width: 100px;
}

Without letting this pass:

p {
  font-size: 16px;
  font-family: monospace;
  .smaller-image {
     width: 100px;
  }
}

[moT01]

You could use some regex in conjunction with something like the old test...
code.match(regex) && $("img").width() < 120;

[ShaunSHamilton]

Added extra width assertion

[moT01]

I think this falls under the code freeze issue - I am still trying to find out if we can merge ones like this that only change tests.

[moT01]

Yea, I'm still not sure on this @SKY020. I think we should just leave it for now. Getting some forum posts?

[moT01]

We are good to merge things in this section that only changes things that won't be translated. Like the tests in this case. So I removed the blocked label.

[ShaunSHamilton]

@moT01

Getting some forum posts?

Yes, but probably only 1 per week:

• https://forum.freecodecamp.org/t/help-with-size-your-images/408038
• https://forum.freecodecamp.org/t/solved-sort-of-stuck-on-the-size-your-images-css-lesson-i-think-its-a-bug/412128

Most recent might be related: https://forum.freecodecamp.org/t/can-anyone-help-me/416296

[moT01]

LGTM 🎉 My only concern is the text that was removed about the browser zoom. @ojeytonwilliams, pretty sure you're involved with the translation efforts - will that cause any problems?

I was able to test this on chome with the zoom at 67% (fails on production, passes with this PR)

[moT01]

I reapplied to blocked label. I think we will get the go ahead on all of them very soon.

[RandellDawson]

@SKY020 You will need to adjust the regex to also allow the correct CSS I have indicated in the comment. This is why we will hopefully have a CSS parser for use in the current curriculum in the near future.

[ojeytonwilliams]

@moT01 sorry, missed your ping. No, it should be fine as far as translation goes. In principle it could have been a problem, but all we're doing is removing a warning (and the necessity for it). Worst case scenario, users end up switching to 100% zoom unnecessarily.

[ShaunSHamilton]

@RandellDawson How about this change of leaving the ; out.

• If the CSS is valid, the image will resize, in any case, and the tests will pass (provided the size is correct)
• If the CSS is not valid, the image will not resize, and the tests will fail.

No need to check if the final property has a closing ; or not.

Examples:

• This passes, image resizes

 .smaller-image{
  background: red;
  width: 100px
}

• This does not pass, image does not resize

 .smaller-image{
  background: red;
  width: 100px
  height: 30%;
}

[RandellDawson]

@RandellDawson How about this change of leaving the ; out.

• If the CSS is valid, the image will resize, in any case, and the tests will pass (provided the size is correct)
• If the CSS is not valid, the image will not resize, and the tests will fail.

No need to check if the final property has a closing ; or not.

Examples:

• This passes, image resizes

 .smaller-image{
  background: red;
  width: 100px
}

• This does not pass, image does not resize

 .smaller-image{
  background: red;
  width: 100px
  height: 30%;
}

With the latest changes, I can leave off the } and still pass the first test:

.smaller-image{
  
  
  width: 100px;
  

[RandellDawson]

Try this change.

[RandellDawson]

Looks good to me now.

[moT01]

LGTM @SKY020 🎉 Thanks for contributing to freeCodeCamp 🎉

[lasjorg]

@SKY020 @RandellDawson @moT01

Did this PR brake the challenge on Safari? Can anyone confirm this?

We are getting forum post about this and when I test it in a VM on lambdatest it fails (I can't really test this any other way). I should say that I'm not sure if it was broken before this PR or not, but the forum threads look to be new, so it seems to points to this PR.

Environment: Safari 10.1 macOS Sierra

https://forum.freecodecamp.org/t/help-here-i-have-tried-everything-above
https://forum.freecodecamp.org/t/bug-in-css-image-sizing/
https://forum.freecodecamp.org/t/i-am-having-problem-with-css-image-size-i-think-i-have-done-the-needful-but-it-is-still-telling-me-to-make-my-image-100px/
https://forum.freecodecamp.org/t/why-do-i-keep-getting-this-wrong

[ShaunSHamilton]

@lasjorg , I cannot confirm this. I have no access to Safari. Two of those campers you linked to were using outdated browsers. So there might be something there.

Otherwise, I cannot remember why we spent so much time making the regex part of the test so robust, only to also include the .width() in the assertion as well...

@moT01 or @RandellDawson If one of you have access to Safari, and are able to test locally with and without the jQuery bit in the assertion. I think that would be the biggest step in debugging.

[lasjorg]

Otherwise, I cannot remember why we spent so much time making the regex part of the test so robust, only to also include the .width() in the assertion as well...

@SKY020 I would not be surprised if the DOM width check was the issue. It certainly wouldn't be the first time a browser didn't give back expected values from DOM querying.

[moT01]

Yes @lasjorg, this is not working in Safari from my testing. The reason for the regex and the jQuery here was for how browsers render the width value. Some browsers give 100, some give something close to it, like 99.9 or something. So the combo test of checking that the size is less than 200 and a regex to see if their code had the css seemed like a way to be sure they did it correctly. Looks like the issue has to do with the regex. I'm getting this...
err: SyntaxError: Invalid regular expression: invalid group specifier name
I think this post explains the issue.

[moT01]

I made a quick PR to hopefully fix it.

[lasjorg]

@moT01 That makes sense. I didn't really look at the regex so I just assumed it might be caused by the width check. But as you have pointed out the lookbehind is not supported in Safari (compatibility table).

Thanks for the quick PR.