Numark Scratch: Manual

[alhadebe]

mapping PR is 4834

[alhadebe]

@ronso0 help please! what's wrong with my svgs?
also the link check failure seems wrong.

[Swiftb0y]

I'm sorry, the scour hook is weird sometimes. Just run it manually on the SVG it complains about and then keep amending the last commit until it passes on CI.

[alhadebe]

I closed this PR by mistake. I'll have a look over the weekend. thanks

[Swiftb0y]

No problem, I figured that. I just wanted to say that you don't really have to worry about the scour hook. I'm looking into switching from scour to svgo which does not seem to have this "instability" (also its still maintained while scour isn't).

[JoergAtGithub]

I see that you had problems to get the SVG images through pre-commit. But changing the format from SVG to PNG is not the right approach.
Do you've pre-commit installed locally?

[alhadebe]

I created a new fresh svg and it seems local & online pre-commit worked this time around.

[ronso0]

Please add a white background to SVGs to ensure it's readable even if a browser uses different main background color (dark/night mode or other reason).

IMO you can then squash the SVG/png commits into one and force-push, if that's okay for all reviewers @JoergAtGithub ?

[ronso0]

strange, pre-commit complains again.
You can simply apply the patch from the check, lines 173-176
https://github.com/mixxxdj/manual/actions/runs/5333920658/jobs/9665031422?pr=492#step:4:173

[alhadebe]

I'll have another look when I get home. I think scour initially removed the white background when optimising. 🤷🏾‍♂️

[ronso0]

Okay, great, pre-commit is happy now!

Now please

... squash the SVG/png commits into one and force-push, if that's okay for all reviewers @JoergAtGithub ?

[Swiftb0y]

lgtm otherwise

[JoergAtGithub]

if that's okay for all reviewers @JoergAtGithub ?

I've not reviewed this PR - feel free to merge this, when you think it's ready

[ronso0]

I've started a text review, but had to leave. I hope to finish this asap

[alhadebe]

Please note I'll need to rebase this manual to 2.4 as the mapping is based on the 2.4 branch. Or the rebase the mapping back to 2.3 to get it out quicker.

[JoergAtGithub]

We updated the JavaScript engine from 2.3 to 2.4. Therefore you would need to change the syntax at various places to backport it to 2.3. Therefore better keep the mapping on target 2.4 and rebase the manual!

[ronso0]

Some ideas to improve the flow and reduce redundancy.

[ronso0]

Thanks, LGTM now.

Waiting for the mapping PR