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

Log entry detail page changed appearance in 235, is it worse? #15111

Closed
AdamWill opened this issue Jan 7, 2021 · 5 comments
Closed

Log entry detail page changed appearance in 235, is it worse? #15111

AdamWill opened this issue Jan 7, 2021 · 5 comments

Comments

@AdamWill
Copy link
Contributor

AdamWill commented Jan 7, 2021

Cockpit version: 235
OS: Any
Page: Logs

In Cockpit 235, the detailed view of a log entry changed appearance a bit. While writing this report, I realized it's actually changed twice in the last year or so. It's basically a view of field identifiers and values. In 235, it looks like this:

235

Note the identifiers and values appear to both be bold, and the identifier font actually looks slightly smaller. (The identifier font is regular and the value font is monospace; this is the same in all three looks). In 234 and back to January (I haven't pinpointed the exact version where it changed before) it looked like this:

logs_entry_detail-20200530

Both identifiers and values are bold, it seems, and the same size. And finally, before around mid-January 2020, it looked like this:

logs_entry_detail-20200124

Back then, the identifiers were bold and the values were regular weight. To me that "old" look was actually the best, and the current one is the worst, but what do others think? Thanks!

@marusak
Copy link
Member

marusak commented Jan 18, 2021

In the current version the label has font-weight: 600.
The value has font-family: monospace,monospace;

Other than that, it is a normal PF4 table. If I turn both of those rules off to make it standard table, it looks like this:
normaltabe

We want monospace font, as some values may come in preformated style, in which case monospace makes it to actually look good. Such example is coredump:
With monospace font:
withmonospace
Without monospace:
nomonospace

The last screenshot you provided is without monospace font.
The middle one is with monospace font, but has labels bolder (and bigger?).
@garrett WDYT, should we make labels bigger? Is there value in doing that?

@garrett
Copy link
Member

garrett commented Jan 18, 2021

Is there a way to have the text be in a proportional font unless it has one of the following?

  • it's more than one line
  • has multiple spaces in a row

Meanwhile, I think it used to have proportional font and I remember working on a PR that made it have a monospaced font at some point in the past. I guess this is a regression, due to PF4 porting. Having it set to fully monospaced (with wrapping when needed, so it doesn't overflow horizontally, aka: white-space: pre-wrap) is fine enough.

@AdamWill
Copy link
Contributor Author

I agree that the values being monospace makes sense, and part of the issue here might just be font rendering quirks (it seems like at this exact font size, with the exact resolution in the test VM, the monospace font being used looks pretty "bold" even though perhaps it's not actually bolded).

@garrett
Copy link
Member

garrett commented Jan 19, 2021

I agree that the values being monospace makes sense, and part of the issue here might just be font rendering quirks (it seems like at this exact font size, with the exact resolution in the test VM, the monospace font being used looks pretty "bold" even though perhaps it's not actually bolded).

It looks bold here too. Regardless, ALL CAPS will always look more prominent. I wanted to make it not all caps, but that was shot down as this is debugging info and it's actually in all caps in the system.

I think this is a good example where we might want to deviate from PF4's suggested styling. Perhaps we can de-empahsize it by making it a slightly lighter color (but still keep AAA contrast).

Perhaps we could have a local override with something like:

.pf-c-description-list__term {
  --pf-c-description-list__term--FontWeight: var(--pf-global--FontWeight--normal);
  color: var(--pf-global--Color--200);
}

Oh. But I just looked and this is still a table, not a PF4 widget (description list). 👀 So this wouldn't work as-is.

Other than that, it is a normal PF4 table.

It's not. It's a very old HTML table with a info-table-ct class... and it's using <td> instead of <th> for the "labels", which makes it quite inaccessible. I'll open a bug to have it ported to PF4.

@marusak
Copy link
Member

marusak commented Feb 15, 2021

Closing in favor of #15170

@marusak marusak closed this as completed Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants