Verbesserte Codeblock-Größe und Farbschema

UX
,
22

I very much agree with this sentiment.

We increased the default font size from 15px to 16px. Changing the code block font size down to 13px is a big shift. Is a couple more characters per line a good trade for readability?

Will inline code styling be updated? At the moment those have different background color, font color, and font family. This is very apparent in posts that interweave a lot of regular text, inline code elements, and code blocks. It makes cross-referencing inline code and block snippets slightly more difficult.

Unrelated to the changes (but related to code blocks) - any ideas for improving the on-hover button icons? At 12px and .7 opacity they’re barely visible (especially when overlapping with code)

image
image452×122 4.9 KB

4 „Gefällt mir“
23

Nice observstion. This was changed due to the buttons being too visible. They were changed to btn-flat but I do see how that may have been too drastic. Maybe this calls for a custom style applied to codeblocks?

1 „Gefällt mir“
24

Should we bump the font size up? I was experimenting with 14px yesterday locally and it’s slightly less jarring compared to the default font.

3 „Gefällt mir“
25 
// this is some commented out code

import Component from "@glimmer/component";
import { action } from "@ember/object";
import didInsert from "@ember/render-modifiers/modifiers/did-insert";
import willDestroy from "@ember/render-modifiers/modifiers/will-destroy";
import { service } from "@ember/service";
import DButton from "discourse/components/d-button";
import bodyClass from "discourse/helpers/body-class";
import concatClass from "discourse/helpers/concat-class";
import ReaderModeOptions from "./reader-mode-options";

export default class ReaderModeToggle extends Component {
  @service readerMode;

  get bodyClassText() {
    if (this.readerMode.isTransitioning) {
      return "reader-mode-transitioning reader-mode";
    } else if (this.readerMode.readerModeActive) {
      return "reader-mode";
    } else {
      return "";
    }
  }

  handleDocumentKeydown(e) {
    if (e.ctrlKey && e.altKey && (e.key === "r" || e.key === "®")) {
      this.readerMode.toggleReaderMode();
    }
  }

  @action
  addEventListener() {
    document.addEventListener("keydown", this.handleDocumentKeydown.bind(this));
  }

  @action
  cleanUpEventListener() {
    document.removeEventListener("keydown", this.handleDocumentKeydown);
  }

  <template>
    {{bodyClass this.bodyClassText}}
    <DButton
      {{didInsert this.addEventListener}}
      {{didInsert this.readerMode.setupWidth}}
      {{willDestroy this.cleanUpEventListener}}
      @action={{this.readerMode.toggleReaderMode}}
      @icon="book-reader"
      @preventFocus={{true}}
      title="Toggle Reader Mode (ctrl + alt + r)"
      class={{concatClass
        "icon"
        "btn-default"
        "reader-mode-toggle"
        (if this.readerMode.readerModeActive "active")
      }}
    />
    {{#if this.readerMode.readerModeActive}}
      <ReaderModeOptions />
    {{/if}}
  </template>
}

Here is the latest update :point_up_2:

8 „Gefällt mir“
26

I really like this latest iteration, @jordan.vidrine!

2 „Gefällt mir“
27

To me the latest update is feeling a lot better, I like the colors and sizing is not fighting with our 16px font for text.

def hello
   puts "hello world"
end

Only minor one is that the grey background still feels a bit drab to me and I prefer a tiny bit lighter. But overall I am feeling quite good about this.

@cvx what is your current take?

1 „Gefällt mir“
28

Maybe we could use a light version of the tertiary color.

image
image793×121 4.56 KB

rgba(var(--tertiary-rgb), 0.058)

2 „Gefällt mir“
29

I did think of using a color from a theme color palette, but we can’t predict what that would be. It might end up terrible wrong :grimacing:

I like the blue though

1 „Gefällt mir“
30

Are you on dark mode or light mode?

The gray I decided to go with is lighter than our previous one (I think). Using var(--primary-50)

1 „Gefällt mir“
31

@sam / @fitzy how are you feeling about living with the changes over the past week?

32

IMO we should merge this into core, it looks so much better now.

The old color scheme is making my eyes hurt on old sites :slight_smile:

3 „Gefällt mir“
34

I am definitely in favour of where we’re at now compared to what we have in core.

However, we lost the change to the max-height in commit 98b2763. Was this intentional? I see it was commented out and then deleted in a subsequent commit.

If so, I can still live with a local override.

1 „Gefällt mir“
35

Yes it was deleted as to remain as close to our current codeblock size as possible.

1 „Gefällt mir“
36

Merging to be done here in this PR → UX: Codeblocks experiment merge by jordanvidrine · Pull Request #29870 · discourse/discourse · GitHub

1 „Gefällt mir“
37

Ok, no worries.

1 „Gefällt mir“
38

It looks like the new padding is only applied to .hljs elements, which means that simple code blocks (with no highlighting) don’t get it:

hello

console.log("test")

This also causes post heights to jump on initial load, because the highlighting (and therefore the .hljs class) is applied asynchronously.

Can we fix it up so that the padding change applies to codeblocks, even without the .hljs class?

6 „Gefällt mir“
39

Could the darker background also be applied, for consistency?

5 „Gefällt mir“
40

Thanks for noticing these! I have updated those styles to apply to code rather than just hljs blocks.

4 „Gefällt mir“
41

I need to add another fix here. The fix I have added also now incorrectly makes inline code blocks render on their own line.

This is an example of an inline codeblock incorrectly rendering.

2 „Gefällt mir“
42

Link now returns 404. Maybe makes sense to point to the PR?
UX: Codeblocks experiment merge by jordanvidrine · Pull Request #29870 · discourse/discourse · GitHub

Also, the team and I absolutely love this change. It looks so damn good! :heart_eyes:

3 „Gefällt mir“