Update for latest TRL + sweetness...

[typhonrt]

Thanks for taking a look. This is using the full capabilities of TRL to bring some great results to Foundry community together.

[lgtm-com]

This pull request fixes 1 alert when merging e746413 into a832ebc - view on LGTM.com

fixed alerts:

• 1 for Unused variable, import, function or class

[arcanistzed]

Using SCSS for this is definitely cleaner than vanilla CSS and better for performance than JS, I kind of wish there was a way to do this while still pulling the data from one single source i.e. src/store/statusData.js which should be the single source of those HSL values ideally

[typhonrt]

Yeah.. There was something funky w/ the programmatic version. I probably spent ~3-4 hours at least trying to get the table colors to look right / better first taking a look at what you tried then eventually settling on the Sass route as it was going to be dependable and do exactly what is encoded in the above snippet. I actually used your code to generate that Sass... No way by hand I decided on those values. Here is the snippet that generated that Sass which was modified from your initial effort:

	const statusColors = {
		X: { hsl: [0, 100, 60] },
		O: { hsl: [45, 90, 60] },
		B: { hsl: [30, 90, 40] },
		G: { hsl: [120, 40, 50] },
		N: { hsl: [200, 60, 50] },
		A: { hsl: [0, 0, 50] },
		U: { hsl: [0, 0, 100] }
	};
// 30 / 50 initially
	function logStyle() {

let text = '&:nth-child(odd) {\n';
		for (const [status, data] of Object.entries(statusColors))
		{
			const { hsl } = data;

text += `	&[data-status="${status}"] {
		background-color: hsla(${hsl[0]}, ${hsl[1]}%, ${hsl[2] + 0}%, ${22 * 4}%);
		&:hover { background-color: hsla(${hsl[0]}, ${hsl[1]}%, ${hsl[2] - 2}%, ${22 * 4.2}%); }
	}\n`;
		}
text += '}\n\n';

text += '	&:nth-child(even) {\n';
		for (const [status, data] of Object.entries(statusColors))
		{
			const { hsl } = data;

text += `	&[data-status="${status}"] {
		background-color: hsla(${hsl[0]}, ${hsl[1]}%, ${hsl[2] + 7 * 0.2}%, ${22 * 3.2}%);
		&:hover { background-color: hsla(${hsl[0]}, ${hsl[1]}%, ${hsl[2] + 7 * 0.6}%, ${22 * 3}%); }
	}\n`;
		}
text += '}';

console.log(text);
	}

	logStyle();

Mind that the above code is ugly / quick hack to generate the Sass. What I have above is tweaked from your initial values as well. You had values well above the thresholds for hsla colors and even some that I generated are, but not by much.

What made the table look much better was adding the inset box shadow when hovering which is just above the Sass block you quoted:

			&:hover {
				box-shadow: inset 0 8px 6px -6px rgba(0, 0, 0, 0.5), inset 0 -8px 6px -6px rgba(0, 0, 0, 0.5);
				box-sizing: border-box;
			}

[arcanistzed]

Note to self to add these back in

[typhonrt]

Is the above really needed though? I get that it is nice to play around w/ "shiny / new" things, but MCC will likely have a long lifetime being available over many potential Foundry versions and not everyone upgrades to the latest and greatest, so there potentially will be straggling users using MCC across a wider swath of Foundry versions. Introducing conditional logic based on which version of Foundry is a really bad code smell. Potentially at least. Up next is using isV11 and so on. Right now the above is relatively benign, but continuing down that path is fraught w/ disaster not only for MCC, but for programming mentality in general.

Certainly do whatever you want of course, but in the future you'll either be the programmer that causes the problems or will be cleaning up after others who introduce conditional logic like this into a codebase. ;P Let me tell you there are far more of the former than the latter. MCC has the opportunity to be a valuable module with a long lifespan; my vision (which may not be yours and that is OK) is that it should be a "lean and mean" module compatibility checking machine.

[arcanistzed]

Why are tooltips and responsiveness disabled?

[typhonrt]

Chart.js unfortunately doesn't have an easy way to make DOM / HTML tooltips. Refer to this issue for conversation: chartjs/Chart.js#7195

There is a custom legend provided in my changes that is much more capable than the legend generated by chart.js. I assume you have at least tried to actually run the app and see what it looks like at this point.

It's sort of possible to create your own HTML tooltips, but not worth it from the amount of code it would require and in this case the scant bit of info actually provided by those tooltips are better represented in the custom legend which gives a real time count of the different status categories.

There is no way to control the separation between the chart and legend with responsive mode enabled and it just doesn't look good or provide a good UX. I spent several hours trying to make it work w/ responsive mode and such first before going custom legend route.

It's certainly possible to add in HTML tooltips with more code, but the custom legend really does well.

Also if you have run the app and checked it out you can see that you can click on the graph and it now sets the category exclusively as well as the legend being clickable and in this case you can actually know the chart & legend is clickable.

[arcanistzed]

I really like what you did here!
Other than some opinionated changes to code formatting and minor stuff like that, this looks good to merge as-is 😄

[lgtm-com]

This pull request fixes 1 alert when merging 7972eef into a832ebc - view on LGTM.com

fixed alerts:

• 1 for Unused variable, import, function or class

[lgtm-com]

This pull request fixes 1 alert when merging 80d017c into a832ebc - view on LGTM.com

fixed alerts:

• 1 for Unused variable, import, function or class

[lgtm-com]

This pull request fixes 1 alert when merging 87b2cef into a832ebc - view on LGTM.com

fixed alerts:

• 1 for Unused variable, import, function or class

[arcanistzed]

I merged this manually into a different branch. Thank you!

[typhonrt]

Cool, but do consider the commits / commit history and PRs are important... Was a bit of work, etc. I'll have to look into the commit squash angle. Glad this is getting out there though.. Good luck w/ the launch.