Refactor/rm computer monitor

[RafaelJohn9]

Changes

Fix #2068

• Removes the Monitor Background Image from the front page
• Makes the different tags that were inside the monitor more visible (changed their colors)

Context

After the remove of the Background image, the monitor tag looks like this:

• I opted to have the tag still to have the name monitor since it contains different details such as latest Git version, Binary links for different operating systems and the release date for the latest Git version.

• One thing I wasn't sure of was whether to remove the images from the repository, would love to hear your thoughts on this.

[dscho]

I have to admit that the new look does not tickle my fancy... But then, I had no problem with the monitor, either. @To1ne do you prefer the new look? If so, I won't object against merging.

[To1ne]

I have to admit that the new look does not tickle my fancy

Well, me neither. For me this makes it more obvious what the latest release is, so that is good. (or maybe my brain was trained to ignore the computer monitor already, that's also possible).

Maybe it's just the empty space that's making it weird. Can you move the other links up? And maybe make the Pro Git section full width?

[To1ne]

Also remove the assets if we don't need them no more.

[dscho]

BTW this is also the reason for the Playwright test failure:

before	after

Notice the upper part of the monitor in the left screenshot?

[dscho]

BTW originally I had meant not to compare screenshots, but to overlay with a really blurry <div> and then measure the pixel brightness, then verify that it is relatively bright for light mode and dark for dark mode. But IIRC I got stuck somewhere along the lines. Maybe it was the "measure pixel brightness".

[dscho]

Whoa, I can't believe that I couldn't make it work back then. It's relatively easy:

const { chromium } = require('playwright');
const fs = require('fs');

(async () => {
  const browser = await chromium.launch(/* uncomment for debugging: { headless: false } */);
  const page = await browser.newPage();
  await page.goto('https://git-scm.com');

  const getBrightness = async () => {
    // Take screenshot as base64
    const screenshot = await page.screenshot({ type: 'png' });
    return await page.evaluate(async (base64) => {
      return new Promise((resolve) => {
        const img = new Image();
        img.src = 'data:image/png;base64,' + base64;
        img.onload = () => {
          const canvas = document.createElement('canvas');
          canvas.width = canvas.height = 1;
          const ctx = canvas.getContext('2d');

          // Draw scaled-down image to average colors
          ctx.drawImage(img, 0, 0, 1, 1);
          const [r, g, b] = ctx.getImageData(0, 0, 1, 1).data;

          // Calculate brightness, for more details, see
          // https://en.wikipedia.org/wiki/Relative_luminance#Relative_luminance_and_%22gamma_encoded%22_colorspaces
          const brightness = 0.2126 * r + 0.7152 * g + 0.0722 * b;
          resolve(brightness);
        };
      });
    }, screenshot.toString('base64'));
  };

  console.log(`Estimated brightness: ${await getBrightness()}`);

  const darkModeButton = page.locator('#dark-mode-button');
  await darkModeButton.click();

  console.log(`Estimated dark brightness: ${await getBrightness()}`);

  // Inject image into page and analyze brightness
  await browser.close();
})();

For me, this prints:

Estimated brightness: 216.4316
Estimated dark brightness: 63.005199999999995

Therefore, I think the best idea would be to change the Playwright tests that want to verify that dark mode is dark, light mode is bright, with a variation of this code, say, dividing the return value by 255 to limit it to the range between 0 and 1, and then assert something like expect(await getBrightness()).toBeCloseTo(0.25, 2) for dark mode (and 0.85 for light mode).

@RafaelJohn9 do you want to run with this and replace the screenshot-based assertions (and likewise these assertions) with this kind of more robust approach?

[RafaelJohn9]

Sure thing 👍,

Will update the tests 🤝

[dscho]

@RafaelJohn9 maybe that update should be split out into its own PR, which could then be fast-tracked?

[RafaelJohn9]

ooh okay,

lemme reset 👍

[dscho]

I didn't mean the deletion of the resources that are no longer referenced. I meant the test update.

[RafaelJohn9]

misunderstood that 😅 ,

Awesome, so should I open another PR referencing the same issue, or should I create a separate issue for it ?

[dscho]

In git-scm.com, we play a bit looser (albeit not as loose as the Git mailing list): We don't require an issue for every PR. Just open another PR with the updated test (and removing the stored screenshots), maybe referencing this finding about the Playwright tests being broken by benign updated as justification.

[jvns]

I've been working on a new potential look for the "Install" page, without the computer monitor. I'm not sure how it should affect the homepage yet. It's not quite ready yet to make a PR but here's a screenshot of the current state:

[dscho]

This is probably how we should present it on the front page, too. Nice, sleek, informative.

[RafaelJohn9]

So uhm,

should I update it to match the deisgn?

[dscho]

So uhm,

should I update it to match the deisgn?

Maybe it would make sense to let @jvns complete the "Install" page and then update the design in the same PR?