Add an ignore option to time-min-milliseconds

[srawlins]

Which issue, if any, is this issue related to?

Fixes #4552

Is there anything in the PR that needs further explanation?

Certainly.

• Previously, this rule considered the value to the shorthand properties to be space-separated lists. It is more correct to view them as comma-separated lists of animations (or transitions) whose values themselves are space-separated lists. This distinction becomes important when ignoring delay values in specific animation (or transition) values. ([MDN: transition, animation)
• Some of the test cases had transposed the timing function and the delay. This is fixed.

[jeddy3]

@srawlins Thanks, this looks great.

Previously, this rule considered the value to the shorthand properties to be space-separated lists. It is more correct to view them as comma-separated lists of animations (or transitions) whose values themselves are space-separated lists.

Good catch spotting and addressing this limitation.

It also highlights another issue with the rule. The location of violations will be incorrect if there are two or more of the same violating times in a value.

The following test will fail:

testRule(rule, {
	ruleName,
	config: [MIN_VALUE],
	reject: [
		{
			code: 'a { animation: foo 0.01s ease 0.8s, bar 0.8s ease 0.01s; }',
			description: 'violating duration and delay of same value',
			warnings: [
				{
					message: messages.expected(MIN_VALUE),
					line: 1,
					column: 20
				},
				{
					message: messages.expected(MIN_VALUE),
					line: 1,
					column: 51
				}
			]
		},
	],
});

The location of the (0.01s) violations will both be 20 because we use indexOf to calculate the offset. It's better we use the value parser's sourceIndex to do this.

if (keywordSets.shorthandTimeProperties.has(propertyName)) {

	let times = [];
	const {durations, delays } = findTime(decl.value);

	if (optionsMatches(options, 'ignore', 'delay')) {
		times = durations;
	} else {
		times = [...durations, ...delays];
	}

	times.forEach(time => {
		if (!isAcceptableTime(time.value)) {
			complain(decl, time.sourceIndex);
		}
	})
}

// duration is first time in shorthand
function findTime(value) {

	let durations = [];
	let delays = [];
	let countInValueList = 0

	valueParser(value).walk((valueNode) => {

		const { type, value } = valueNode;

		// start of new value in the list?
		if (type === 'div' && value === ",") {
			countInValueList = 0;
			return
		}

		if (type !== 'word') return;
		if (!valueParser.unit(value)) return;

		if (countInValueList === 0) {
			durations.push(valueNode);
		} else {
			delays.push(valueNode);
		}

		countInValueList++;

	})

	return { durations, delays };
}

I'm sure the findTime function can be better written, and we'd probably want to extract it into a util as we do with findFontFamily and findListStyleType.

However, this problem existed before this pull request. It's just more noticeable now as it's likely two or more of the same time will appear in a list of animations/transitions than a single animation/transition.

I'll mark as approved and I can create a follow-up issue to address the location issue. Unless you want to tackle it in this pull request?

[srawlins]

Wow thanks for the head start on the bug about violation locations. But I think I'd prefer to tackle that in a new PR.

[jeddy3]

But I think I'd prefer to tackle that in a new PR.

Sounds good to me. I've created a new issue for it at #4751.

[jeddy3]

Changelog:

• Added: ignore:["delay"] to time-min-milliseconds (#4743).

@srawlins Thanks again for the pull request. Apologies for the delay in getting it merged. Some big chunks of work were happening in the background.