fixed keyboard shortcuts on MacOS [#7892]

[bamanczak]

Fixes #7892

Here's the Pull Request Doc
https://firefox-dev.tools/debugger.html/docs/pull-requests.html

Summary of Changes

• Removed spaces from formatKeyShortcut function for MacOS consistency

Test Plan

1. I verified manually that the fix updated the shortcuts on the initial load of debugger
2. I run yarn test (Tests: 10 skipped, 1649 passed, 1659 total)

[errietta]

looks good

[errietta]

The test failures are unrelated, there's #7752 that should fix that

[darkwing]

Nice work. I'd like others to weigh in here; the key representations look "bunched" or too close to me:

I wonder if some letter-spacing would help out here. @violasong @fvsch

[fvsch]

See #7892 (comment) for my comments about the font family.

For spacing, we could use some letter spacing for macOS only. Or we could use thin spaces:
http://www.fileformat.info/info/unicode/char/2009/index.htm
http://www.fileformat.info/info/unicode/char/200a/index.htm
http://www.fileformat.info/info/unicode/char/202f/index.htm

[violasong]

Changing the font-weight to normal may fix the bunched feeling.

[bamanczak]

So right now we have this:

After trying out the suggestion from #7892 comment, it looks like this:

Which looks better if you ask me :)
But as I understand from the comment it would require additional testing? I would need some guidance to make sure we won't introduce any weird regression.

Changing font-weight makes no effect.

[fvsch]

Applying the styles only to macOS requires a selector like:

:root[platform="mac"] .welcomebox .shortcutKey {
  ...
}

Which works for the Debugger in Firefox, but not in "Launchpad" mode (the HTML element doesn't have that attribute). It looks like instead the platform attribute is added to a div in the body, so the relevant selector would become:

:root[platform="mac"] .welcomebox .shortcutKey,
.launchpad-root[platform="mac"] .welcomebox .shortcutKey {
  ...
}

@darkwing, do you know if there is some automation in place that converts the first selector form to the second one? Or do we need to write both?

[fvsch]

In launchpad mode I'm seeing styles like this, which seem to come from mozilla-central but the selector was tweaked:

:root.html [platform="linux"] {
    --monospace-font-family: monospace;
}

Original:

:root[platform="linux"] {
    --monospace-font-family: monospace;
}

[violasong]

Ah, my font-weight comment was based on David's screenshot but looks like your version is different. Latest version looks good but I miss the bolding of the shortcuts :)

[fvsch]

If we use the mac system font we can probably use font-weight: 500 or font-weight: 600 to get a semibold variant.

[darkwing]

Lots of good discussion here! @fvsch can you summarize the change to be made in one comment?

[bamanczak]

I went through the comments and implemented them in a suggested way. Right now the welcomebox looks on Mac as follows:

Which I think is nice :)

[fvsch]

This rule should be placed after the styles for .welcomebox .shortcutKey, to keep a logical order.

[bamanczak]

Do you think now it's the correct place? :)

[fvsch]

Looks like it. :)

[fvsch]

So I'm not very fond of the monospace font here. It looks like with font-family: monospace on a mac, Firefox may end up using Courier, which is not something we have elsewhere in DevTools. The --monospace-font-family CSS variable is actually defined so that we can specify a specific monospace font on each platform, so we shouldn't override it, even locally in some branch of the DOM.

It would be more interesting here IMO to try to use San Francisco, which seems to be the font used in the issue's screenshots. According to Can I Use, Firefox for Mac supports the -apple-system on macOS. For Chrome, it seems that the system-ui value is supported. So maybe something like:

[platform="mac"] .welcomebox .shortcutKey {
  font-family: system-ui, -apple-system, sans-serif;
  font-weight: 600;
}

[bamanczak]

This solutions seemed fine both on Chrome and FF. I also changed font-weight to 500, as it looks better.

[fvsch]

@bamanczak Can you show updated screenshots of the result on Mac, in Firefox and Chrome?
If you think a different font-weight works better, feel free to propose that as well.

[bamanczak]

As requested, screenshot from Chrome:

And from firefox

[darkwing]

Thank you for the update @bamanczak ! We'll have a look!

[darkwing]

I love this @bamanczak ! Thank you!