CSS: Rework custom breakpoint CSS

[garrett]

trafficstars

This might fix #17556. Or at least address it somewhat?

[garrett]

I tried removing the breakpoint inclusions everywhere, but it turns out whenever a breakpoint is used, the file needs to be included. And whenever a PF component is imported in the CSS, it also needs the breakpoints file included.

If we do move the PF component includes into the JSX, I think we'll need to also include the breakpoints overriding before the component inclusion.

[jelly]

With my current main:

With this PR:

[garrett]

All the places where we import from PatternFly in our CSS files:

(The selected one happens to be different in the file with an unecessary ../../node_modules/ in it that I removed locally.)

[garrett]

Here's a look at the utility classes:

Bytes	File
2756	'node_modules/@patternfly/patternfly/utilities/Accessibility/accessibility.css'
1894	'node_modules/@patternfly/patternfly/utilities/Alignment/alignment.css'
15924	'node_modules/@patternfly/patternfly/utilities/BackgroundColor/BackgroundColor.css'
1923	'node_modules/@patternfly/patternfly/utilities/BoxShadow/box-shadow.css'
3878	'node_modules/@patternfly/patternfly/utilities/Display/display.css'
19279	'node_modules/@patternfly/patternfly/utilities/Flex/flex.css'
906	'node_modules/@patternfly/patternfly/utilities/Float/float.css'
16366	'node_modules/@patternfly/patternfly/utilities/Sizing/sizing.css'
108130	'node_modules/@patternfly/patternfly/utilities/Spacing/spacing.css'
33411	'node_modules/@patternfly/patternfly/utilities/Text/text.css'
204467	Total

It's 204 kb. I suppose that's a little too weighty to just import by default. Therefore, I think the best approach when using a utility class is to import the CSS (not SCSS, as that would require importing variables and other dependencies to compile to the same resulting CSS) on the page. Hopefully WebPack would de-duplicate the CSS if it's included by multiple JSX files on the same page.

[garrett]

So the utilities have a ton of breakpoints. Some are not needed, but others let you add a utility on some breakpoints but not others. (Ugh.) But they're PF's wrongly defined breakpoints, not our breakpoints (which are different because we use an iframe). Which means the compiled CSS isn't correct for us, if we ever use a utility only for some breakpoints and not others. :cursing_face:

I tried adding SCSS to the JSX, but it doesn't really work:

import "_breakpoints.scss";
import "@patternfly/patternfly/sass-utilities/mixins.scss";
import "@patternfly/patternfly/utilities/Text/text.scss";

There are still some issues, like:

ERROR in ./node_modules/@patternfly/patternfly/utilities/Text/text.scss
Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
ModuleBuildError: Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
SassError: Undefined mixin.
   ╷
25 │ @include pf-utility-builder($pf-u-font-family-options);
   │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I wonder if this is related.... looking into it:

https://stackoverflow.com/questions/42308421/cant-resolve-scss-files-in-jsx-using-import

[garrett]

[garrett]

webpack's null-loader (which is deprecated, BTW), prevents loading those files... so they can be loaded by the SCSS build stack instead of webpack, to prevent duplication.

What would be ideal is to get webpack to load the overridden breakpoints while it is telling SASS to compile the SCSS for the components, so we don't have to jump through hoops and can just use PF React components in JSX directly, with correct breakpoints as well.

However, that still won't address the static PF CSS.

[garrett]

We have 2 main problems:

1. React and SASS compilation no nothing about each other.
2. Both of these compile steps, import (JSX) and @import (SASS) copy the CSS, regardless of whether import has been used before (even in other parts of included SCSS files elsewhere on the page). WebPack might de-duplicate JSX import (this needs to be tested), but it definitely does not touch SASS @import.

Both of these cause multiple copies of CSS to happen.

Redundant CSS is:

• bad for bloat (takes longer to send, parse, etc.),
• hurts debugging
• messes up specificity (meaning CSS won't be applied as expected, and this is especially a problem for dark mode)

[garrett]

Rebased. Although, I think we need to rework our build system and rewrite the breakpoints at that level (by changing the magic pixel values.... and drop our custom stuff).

[garrett]

I think this attempt is no longer needed, thanks to the esbuild refactor: https://github.com/cockpit-project/cockpit/pull/18447