systemd: White widget backgrounds in the journal filter toolbar

[mvollmer]

The secondary button should not be transparent.

At the same time, improve the existing CSS that does the same job for
menu toggles.

The toolbar already has its own super custom styling, which I had
missed earlier. Let's move our tweaks for the menu toggles into that
section and do them by setting the appropriate PF variable.

Also, we can remove the FIXME part.

[martinpitt]

Technical review 👍 . Happy to give a formal ack if you already agreed on this with Garrett someplace else. Thanks!

[martinpitt]

@mvollmer would you mind putting a before/after screenshot? that may make it easier for @garrett to review.

[garrett]

I agree with this sentiment and the CSS looks like what I'd suspect. I'd be able to approve it with a screenshot. 😉

[garrett]

Also: Note: This would affect all buttons, regardless of context (what other widgets they're in), so it might have some unknown effects in some places.

It's probably correct, however.

[mvollmer]

The hostswitcher has changed in light mode:

as opposed to earlier:

This does increase the contrast, and now the buttons have the same background color as the text input box. So I guess it's correct in that sense and looks as intended.

[mvollmer]

The rest of the pixel changes are as expected. For example, the secondary buttons on the Networking page now look like this in dark mode:

as opposed to:

[garrett]

That actually decreases contrast between the background and the text, not increases it.

(referring to #21389 (comment))

The way PatternFly is designed is to have secondary buttons inherit the background color.

Buttons are incorrect in all the screenshots. IIRC, the goal was to ensure that there is enough contrast between the text and background in certain circumstances (where the background that is bleeding through and text does not have enough contrast).

[mvollmer]

That actually decreases contrast between the background and the text, not increases it.

Ah, I misunderstood which contrast we want to increase: I thought buttons should have a different background than their surroundings.

The way PatternFly is designed is to have secondary buttons inherit the background color.

And we should keep that?

It all started with this comment and screenshot:

You said:

the widgets should all have a white background there, including the secondary button; they need contrast... the text on grey isn't enough contrast, and additionally the grey on the widgets make them look disabled

From that I thought that widgets should get the primary background color (white in light mode), including secondary buttons. I completely missed the "text on grey" part. :-(

So in this PR it looks like this in light mode:

And this is dark mode now:

In both modes, the secondary button has the same background as the text input (for example).

I am happy to just close this PR, of course, but if you think we should change how secondary buttons look, I am happy to do that as well. :-) But I can't tell right now what should be changed exactly.

[garrett]

Yeah, this is clearly wrong:

The dropdowns and the button should have the white background. The grey background makes them not look like widgets, does not have enough contrast between the text and the background, and even makes the button look disabled. Those are all things that need fixing, and why it has a white background on main (unless that was changed recently and is then a big usability and accessibility bug if so) and should always have a white background.

I'm pretty sure I said elsewhere that this should not happen globally, but only in cases like this. It's fine to have a fix for these widgets in the general toolbar widget like this, but it's not OK to just globally change the background of the widgets everywhere.

[mvollmer]

The dropdowns and the button should have the white background.

Ok! What about dark mode? How should that look in the specific case of the journal filter header?

I'm pretty sure I said elsewhere that this should not happen globally, but only in cases like this.

Yes, I agree.

[garrett]

Ok! What about dark mode? How should that look in the specific case of the journal filter header?

Good question. Which component is providing that grey background? I did a look through the PF5 site through all components and patterns to find a reference, but can't find it. What is it specifically?

Perhaps we're doing something an old, unsupported way or something out of spec. I think this page was even pre-PatternFly at some point? Perhaps we need to change the widgets and the look it's using overall? Perhaps this is why it's weird?

[mvollmer]

Which component is providing that grey background?

It's a Toolbar, with our own styling:

.pf-v5-c-toolbar {
  --pf-v5-c-toolbar--BackgroundColor: var(--pf-v5-c-page__main-section--BackgroundColor);

  // Make toolbar stretch to all the available space and wrap up to two lines
  .pf-v5-c-toolbar__group:nth-child(3) {
    flex-grow: 1;
  }

  // Make text filter stretch to all the available space
  .pf-v5-c-toolbar__item.text-search, .journal-filters-grep {
    flex-grow: 1;
  }

  .text-help {
    padding-inline-start: var(--pf-v5-global--spacer--xs);
  }

  // Hide filters from advanced search dropdown entries which already exist in the toolbar
  .journal-filters-grep .pf-v5-c-panel__main-body {
    .pf-v5-c-form__group:nth-child(5), .pf-v5-c-form__group:nth-child(6) {
      display: none;
    }
  }

  .pf-v5-c-toolbar__expandable-content.pf-m-expanded .pf-v5-c-divider {
    display: none;
  }

  // FIXME: When porting the selects to the PF5 select implementation drop this
  .pf-v5-c-toolbar__item {
    align-self: center;
  }
}

I wasn't aware that toolbars normally don't have a grey background...

Oh, there is a FIXME that we can now act on. :)

[mvollmer]

Ok, I have redone this. The CSS changes now only apply to the journal filter toolbar, and I hope I have also found a better way get the desired effect by setting some PF variables.

Dark mode is unchanged for now.

[mvollmer]

Pixel changes are as expected: https://github.com/cockpit-project/pixel-test-reference/compare/c83ced24f3d8a9d0279808014598b9a85893d061..246a85aef9d47b950a61ac9327c0cd7cd1f0cdc8