Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent formating for shortcuts #3249

Closed
YeldhamDev opened this issue Mar 6, 2020 · 9 comments · Fixed by #3257
Closed

Inconsistent formating for shortcuts #3249

YeldhamDev opened this issue Mar 6, 2020 · 9 comments · Fixed by #3257

Comments

@YeldhamDev
Copy link
Member

The way keyboard shortcuts are displayed in docs is very inconsistent, some examples:

  • Ctrl D
  • Ctrl+D
  • Ctrl + D
  • (Ctrl + D)
  • Ctrl + D
  • Ctrl D (Which doesn't show in the actual docs.)
@YuriSizov
Copy link
Contributor

The last one is the most visually pleasing, so I'd stick with that.

We need to consider, that macOS often has different default shortcuts than other platforms (#3244), which is not displayed properly throughout the docs. So if we do some big change to shortcut formatting, it makes sense to fix both problems together (with some formatting macros probably?).

@YeldhamDev
Copy link
Member Author

The last one is the most visually pleasing, so I'd stick with that.

The problem is that, like I said, it doesn't actually show in the docs, appearing just as <kbd></kbd> instead.

@YuriSizov
Copy link
Contributor

Okay, so I'd stick with that, and investigate, why does it not work currently. 😏

I'll take a look at that problem later.

@YuriSizov
Copy link
Contributor

So, I poked around, and seems that <kbd> tag is working perfectly fine, but it is almost never used. Looking through the codebase, only three articles refer to it:

image

First one being the same as @YeldhamDev described, because somebody simply put the html tag as a plain text. This does not work, obviously, and the end result is thus:

image
from GDScript style guide

The other two articles are properly using the :kbd: macro like so:

:kbd:`F1`

This results in a correctly generated HTML, but there are no CSS rules associated with it, so the end result is unnoticable (only font differs, there is a rule that sets it to monospaced):

<kbd class="kbd docutils literal notranslate">F1</kbd>

image
from Control the game’s UI with code

There is this PR in RTD repo from 2018, but Godot Docs don't seem to have it. I have no idea, if and how theme changes are pulled into Godot Docs, though.

On a bright side, this can be easily fixed with custom.css rule anyway, because HTML is already there. Here is an example of GitHub styles applied directly to the light theme and my quick attempt at adjusting it for the dark theme:

Light Theme

image

Source:

kbd, .kbd {
    display: inline-block;
    padding: 3px 5px;
    font-size: 11px;
    line-height: 10px;
    color: #444d56;
    vertical-align: middle;
    background-color: #fafbfc;
    border: 1px solid #d1d5da;
    border-radius: 3px;
    box-shadow: inset 0 -1px 0 #d1d5da;
}

Dark Theme

image

Source:

kbd, .kbd {
    display: inline-block;
    padding: 3px 5px;
    font-size: 11px;
    line-height: 10px;
    color: #98cdf5;
    vertical-align: middle;
    background-color: #595b5d;
    border: 1px solid #43484c;
    border-radius: 3px;
    box-shadow: inset 0 -1px 0 #1e2023;
}

Now, I can make a PR, that adds this and look through sources replacing every instance of a shortcut being mentioned with a proper macro, but I have to know, if we are going with

  • Ctrl + Alt
  • Ctrl - Alt
  • Ctrl Alt
  • Ctrl+Alt
  • Ctrl-Alt
  • Ctrl Alt
  • something else?

And there is a withstanding issue about macOS shortcuts, though if no fancy solution is available we can at least mark them down like so:

Press F1 (Win, Linux) / Alt 1 (macOS) to open 2D editor

or so

Press F1 (or Alt 1 on macOS) to open 2D editor

@Calinou
Copy link
Member

Calinou commented Mar 9, 2020

Thanks for the detailed writeup 🙂

I would go for the <kbd>Ctrl + Alt + T</kbd> syntax as it's fairly terse (only one tag for the whole shortcut). It displays like this: Ctrl + Alt + T

@YuriSizov
Copy link
Contributor

YuriSizov commented Mar 9, 2020

There are a couple of details, that may need more discussion.

  • Control or Ctrl?
  • Command or Cmd?
  • What about "Windows"? Should that change to "Meta"?
  • What about "Arrow Keys"?
  • What to do with mouse combinations, like Shift+Right-click?

I prefer short versions for "Control" and "Command". Keyboard + Mouse combinations can be written around with something like "Press Right-click while holding Shift". Don't know about the rest.

Edit: I guess, Windows can be literally Windows sometimes. But should it be Windows or Win?
Edit 2: Arrow keys present another issue as well. When using in combination, everything looks okay: Ctrl + Up. However, when its only an arrow key, it looks a bit weird: Left. Or is this fine?

@Calinou
Copy link
Member

Calinou commented Mar 9, 2020

@pycbouh I'd go for Ctrl and Cmd respectively. Keep using Windows as it's the more commonly known name for that key. For arrow keys, use Up Arrow. Lastly, for mouse combinations, I'd use Shift + Right-click.

@akien-mga
Copy link
Member

There is this PR in RTD repo from 2018, but Godot Docs don't seem to have it. I have no idea, if and how theme changes are pulled into Godot Docs, though.

That's weird. We should be using the latest RTD image, so unless that PR was reverted, I'd expect that we should be running a recent version of sphinx_rtd_theme: https://github.com/godotengine/godot-docs/blob/master/.readthedocs.yml

@akien-mga
Copy link
Member

There is this PR in RTD repo from 2018, but Godot Docs don't seem to have it. I have no idea, if and how theme changes are pulled into Godot Docs, though.

That's weird. We should be using the latest RTD image, so unless that PR was reverted, I'd expect that we should be running a recent version of sphinx_rtd_theme: https://github.com/godotengine/godot-docs/blob/master/.readthedocs.yml

Ah, it was merged into a branch, it never made it to their master/release branch: readthedocs/sphinx_rtd_theme#677 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants