Treat CSS aspect-ratio, scale & a few others as unitless

[ukcbajr]

Found an issue when using .css('aspect-ratio', x);

If x is a number, 'px' seems to be added, causing the attribute to not be set.

Looks like 'aspectRatio' needs to be added to cssNumber definition.

My current work-around is to convert x to a string, e.g. use .css('aspect-ratio', x+'');

[mgol]

@ukcbajr Thanks for the report. I transferred it from jQuery UI to jQuery as the issue is not related to UI.

Would you like to submit a PR? You'd need to add the property to

jquery/src/css.js

Lines 213 to 234 in 0acbe64

	cssNumber: {
	"animationIterationCount": true,
	"columnCount": true,
	"fillOpacity": true,
	"flexGrow": true,
	"flexShrink": true,
	"fontWeight": true,
	"gridArea": true,
	"gridColumn": true,
	"gridColumnEnd": true,
	"gridColumnStart": true,
	"gridRow": true,
	"gridRowEnd": true,
	"gridRowStart": true,
	"lineHeight": true,
	"opacity": true,
	"order": true,
	"orphans": true,
	"widows": true,
	"zIndex": true,
	"zoom": true
	},

on the 3.x-stable branch. On the main branch we already changed the approach and we're only appending px to a finite list of properties so any new CSS property will not get px appended.

[mgol]

Also, we'll need tests for this new property at https://github.com/jquery/jquery/blob/3.x-stable/test/unit/css.js.

[ukcbajr]

Hi I'm really not familiar with github. What's a PR? How would I add a property to the file you mentioned? No idea about your tests at all. Apologies.

[mgol]

@ukcbajr PR is Pull Request. OK, if you're not familiar with GitHub, we can implement this fix by ourselves. Unless you'd like to learn how to use GitHub and how to contribute to jQuery (including writing tests) then I can guide you through the process. Let me know!

[ukcbajr]

Yes I should learn. Happy to follow guidance!

[mgol]

@ukcbajr OK, cool! The first step might be to read the GitHub docs on managing branches & creating pull requests:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches
and then to read jQuery own docs on contributing: https://contribute.jquery.org/commits-and-pull-requests/

The easiest way to run tests locally now is (once you have a local jQuery clone and a branch for the fix prepared per the guide above) to invoke:

npm install

and then:

npm run test:browser

Then you can re-run the last command after writing code changes & the tests to make sure they pass.

That's the first step. If you want to iterate faster on your tests, you can install grunt-cli globally:

npm install -g grunt-cli

and then enter debug mode:

grunt karma:chrome-debug

This will open Chrome with a new user profile with a Debug button - you should click it; this will lead you to the QUnit (our testing library) UI where you can e.g. select modules to test, re-run single failing tests, etc. To learn more about QUnit, you can read the page of that project, e.g.: https://qunitjs.com/intro/#in-the-browser

Let me know if anything's unclear!

[ukcbajr]

Ok will work on this. FYI I find the language very confusing. E.g. "If you push new commits to the branch from which you initiated the pull request, the pull request will automatically be updated."Why is it called a pull request and not a push request?Sent from my Verizon, Samsung Galaxy smartphone -------- Original message --------From: Michał Gołębiowski-Owczarek ***@***.***> Date: 12/19/22 10:14 AM (GMT-08:00) To: jquery/jquery ***@***.***> Cc: John Bell ***@***.***>, Mention ***@***.***> Subject: Re: [jquery/jquery] css 'aspect-ratio' not treated as unitless. (Issue #5179) @ukcbajr OK, cool! The first step might be to read the GitHub docs on managing branches & creating pull requests: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches and then to read jQuery own docs on contributing: https://contribute.jquery.org/commits-and-pull-requests/ The easiest way to run tests locally now is (once you have a local jQuery clone and a branch for the fix prepared per the guide above) to invoke: npm install and then: npm run test:browser Then you can re-run the last command after writing code changes & the tests to make sure they pass. That's the first step. If you want to iterate faster on your tests, you can install grunt-cli globally: npm install -g grunt-cli and then enter debug mode: grunt karma:chrome-debug This will open Chrome with a new user profile with a Debug button - you should click it; this will lead you to the QUnit (our testing library) UI where you can e.g. select modules to test, re-run single failing tests, etc. To learn more about QUnit, you can read the page of that project, e.g.: https://qunitjs.com/intro/#in-the-browser Let me know if anything's unclear! —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[ukcbajr]

OK I forked the repository, identified the file and made the change in css.js. (Used browser and edited in github.com

It's going to take a while to figure out the test part. Perhaps you can do this? I'll create a pull request with the 1-line edit I made.

[mgol]

We’re not in a hurry, you can spend some time on figuring out tests. For that you need to have this branch available locally, though.

[ukcbajr]

Ok thanks.One issue - I can clone the forked repository I created to my local machine and see the main files, but not the branch I created. How (on my windows desktop) do I see the set of files associated with just a branch so I can test? Have not found any documentation on this, so if you can send me a link I'd appreciate it.Sent from my Verizon, Samsung Galaxy smartphone -------- Original message --------From: Michał Gołębiowski-Owczarek ***@***.***> Date: 12/21/22 8:36 AM (GMT-08:00) To: jquery/jquery ***@***.***> Cc: John Bell ***@***.***>, Mention ***@***.***> Subject: Re: [jquery/jquery] css 'aspect-ratio' not treated as unitless. (Issue #5179) We’re not in a hurry, you can spend some time on figuring out tests. For that you need to have this branch available locally, though. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[mgol]

@ukcbajr First, fetch all branches:

git fetch --all --tags -p 

Then, you can check out your branch:

git checkout 3.6-stable-css-aspectRatio-force-numeric

You can make changes there, then add them to the index:

git add -p

This will be asking you for changes to add to the index for committing. You'll likely want all of them. Then, you'll need to commit:

git commit -m 'Insert commit message here'

and push:

git push

[ukcbajr]

Ok I've been using the github desktop app, not command line. Do I have to use command line for this?

[mgol]

You should be able to use the GitHub desktop app as well. I'm not very familiar with the app so I can't help much but I assume it should have everything needed to work with PRs like that built-in.

The important point here is that you need to switch from the main branch to your 3.6-stable-css-aspectRatio-force-numeric one.

Also, please read Timmy's comment on your original PR: #5183 (comment). It looks like you started from the 3.6-stable branch and we need you to start from 3.x-stable.

[mgol]

For the record, the PR we're talking about is at #5184.

[mgol]

PR: #5233

[mgol]

Fixed by #5233.