Mejorado el tamaño del bloque de código y esquema de colores

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 Me gusta
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 me gusta
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 Me gusta
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 Me gusta
26

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

2 Me gusta
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 me gusta
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 Me gusta
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 me gusta
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 me gusta
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 Me gusta
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 me gusta
35

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

1 me gusta
36

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

1 me gusta
37

Ok, no worries.

1 me gusta
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 Me gusta
39

Could the darker background also be applied, for consistency?

5 Me gusta
40

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

4 Me gusta
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 Me gusta
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 Me gusta