#51187 cockpit: Stop importing cockpit's base1/patternfly.css
Closed: wontfix 4 years ago by mreynolds. Opened 4 years ago by kkoukiou.

This is deprecated API and will be dropped at some point, in of projects shipping their own CSS.

Follow https://github.com/cockpit-project/starter-kit/blob/master/webpack.config.js approach on how to use PF CSS.


Metadata Update from @mreynolds:
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Issue priority set to: critical
- Issue set to the milestone: 1.4.4

4 years ago

Please see the announcement for some details and links.

Please see the announcement for some details and links.

Yes I tried all the suggestions, replaced the webpack config file, etc, but it just breaks our plugin. We have dependency issues, and all kinds of other problems. I spent quite a bit of time trying to get it to work. Part of the problem on our side, I think, is that we use the PF 4 css that comes with Cockpit, but we still use PF React 3.

PF React 4 does not have all the PF 3 components that we use, like TreeView. So this has delayed us from completely going to PF 4 (html and react). I'm hoping that once we get off PF react 3 and on to 4 then that will allow us to ship our own CSS via the instructions provided by Cockpit.

I did NOT try the podman commit yet - this is the one for mixed PF envs like ours, so hopefully this will work for us:

https://github.com/cockpit-project/cockpit-podman/commit/86fa1ee8b84

Metadata Update from @mreynolds:
- Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1851981

4 years ago

@mreynolds : Indeed, cockpit's code and cockpit-podman are in the same boat -- they still have lots of PF3 code as well. I suggest to not try and port everything to PF4 at the same time, this will be quite an enormous undertaking. That's the reason why base1/patternfly.css has PF3 classes with tons of cockpit modifications so that they look like PF4. With that, you can replace the components one by one without breaking the design each time.

So the cockpit-podman commit ought to do that, it builds both PF3 and PF4.

@mreynolds : Indeed, cockpit's code and cockpit-podman are in the same boat -- they still have lots of PF3 code as well. I suggest to not try and port everything to PF4 at the same time, this will be quite an enormous undertaking. That's the reason why base1/patternfly.css has PF3 classes with tons of cockpit modifications so that they look like PF4. With that, you can replace the components one by one without breaking the design each time.
So the cockpit-podman commit ought to do that, it builds both PF3 and PF4.

Thanks @martinpitt - we'll be looking into this in our next sprint (starting this week)

@martinpitt - having issues. I did everything podman does. I don't get any errors when building the UI, but looks like it's not finding any patternfly styles

Screenshot_from_2020-06-30_14-35-43.png

It should look like this:

Screenshot_from_2020-06-30_14-37-45.png

I don't see any errors, and nothing stands out in the consoie log. It's like patternfly is not even present, console inspect shows PF is not being used. Not sure what I am missing :-(

I see this during building:

uilt at: 06/30/2020 2:50:27 PM
                                  Asset       Size  Chunks                   Chunk Names
                        css/_fonts.scss   1.45 KiB          [emitted]        
                       css/branding.css   73 bytes          [emitted]        
                             css/ds.css     11 KiB          [emitted]        
          css/patternfly-4-cockpit.scss  633 bytes          [emitted]        
            css/patternfly-cockpit.scss  449 bytes          [emitted]        
css/patternfly-overrides-variables.scss   2.58 KiB          [emitted]        
          css/patternfly-overrides.scss   16.1 KiB          [emitted]        
                              index.css    463 KiB   index  [emitted]        index
                          index.css.map   1.23 MiB   index  [emitted] [dev]  index
                             index.html  517 bytes          [emitted]        
                               index.js   9.68 MiB   index  [emitted]        index
                           index.js.map   9.81 MiB   index  [emitted] [dev]  index
                          manifest.json  208 bytes          [emitted]        
Entrypoint index = index.css index.js index.css.map index.js.map
[0] multi ./src/./index.es6 28 bytes {index} [built]
[./src/css/branding.css] 39 bytes {index} [built]
[./src/css/ds.css] 39 bytes {index} [built]
[./src/css/patternfly-cockpit.scss] 39 bytes {index} [built]
[./src/database.jsx] 57.3 KiB {index} [built]
[./src/ds.jsx] 31 KiB {index} [built]
[./src/dsModals.jsx] 51.9 KiB {index} [built]
[./src/index.es6] 298 bytes {index} [built]
[./src/lib/notifications.jsx] 10.8 KiB {index} [built]
[./src/lib/tools.jsx] 4.07 KiB {index} [built]
[./src/monitor.jsx] 54.1 KiB {index} [built]
[./src/plugins.jsx] 25.3 KiB {index} [built]
[./src/replication.jsx] 51.2 KiB {index} [built]
[./src/schema.jsx] 42.9 KiB {index} [built]
[./src/server.jsx] 14.4 KiB {index} [built]
    + 1521 hidden modules
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--11-1!node_modules/string-replace-loader/index.js??ref--11-2!node_modules/sass-loader/dist/cjs.js??ref--11-3!src/css/patternfly-cockpit.scss:
    Entrypoint mini-css-extract-plugin = *
    [./node_modules/css-loader/dist/cjs.js?!./node_modules/string-replace-loader/index.js?!./node_modules/sass-loader/dist/cjs.js?!./src/css/patternfly-cockpit.scss] ./node_modules/css-loader/dist/cjs.js??ref--11-1!./node_modules/string-replace-loader??ref--11-2!./node_modules/sass-loader/dist/cjs.js??ref--11-3!./src/css/patternfly-cockpit.scss 1.71 MiB {mini-css-extract-plugin} [built]
        + 1 hidden module
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--12-1!node_modules/sass-loader/dist/cjs.js??ref--12-2!src/css/branding.css:
    Entrypoint mini-css-extract-plugin = *
    [./node_modules/css-loader/dist/cjs.js?!./node_modules/sass-loader/dist/cjs.js?!./src/css/branding.css] ./node_modules/css-loader/dist/cjs.js??ref--12-1!./node_modules/sass-loader/dist/cjs.js??ref--12-2!./src/css/branding.css 485 bytes {mini-css-extract-plugin} [built]
        + 1 hidden module
Child mini-css-extract-plugin node_modules/css-loader/dist/cjs.js??ref--12-1!node_modules/sass-loader/dist/cjs.js??ref--12-2!src/css/ds.css:
    Entrypoint mini-css-extract-plugin = *
    [./node_modules/css-loader/dist/cjs.js?!./node_modules/sass-loader/dist/cjs.js?!./src/css/ds.css] ./node_modules/css-loader/dist/cjs.js??ref--12-1!./node_modules/sass-loader/dist/cjs.js??ref--12-2!./src/css/ds.css 24.6 KiB {mini-css-extract-plugin} [built]
        + 1 hidden module

Any suggestions would be appreciated? I'll keep trying to figure out what is going wrong...

@mreynolds : Could you push your current work somewhere? I'm happy to take a look. @kkoukiou will be much better at this, but she's on PTO right now.

@martinpitt - thanks! This is probably more than you were asking for, but here are all the details :-)

Install 389-ds-base* cockpit-389-ds

Get the source:

cd /home/USERNAME/source

git clone https://git@pagure.io/389-ds-base.git

cd 389-ds-base

And apply this patch

0001-Phase-1-don-t-import-PF-from-cockpit.patch

Then when I test locally I do:

mkdir ~/.local/share/cockpit/389-console
  • Link your workspace directory

    ln -s /home/USERNAME/source/389-ds-base/src/cockpit/389-console/dist ~/.local/share/cockpit/389-console

  • Then run the build & run script. This builds the UI and "watches" it for changes

    cd /home/USERNAME/source/389-ds-base/src/cockpit/389-console/
    ./buildAndRun.sh

Then I log into cockpit as myself (with sudo rights of course) and goto "389 Directory Server"

Thanks, I can reproduce this.

The first oddity is that you put the scss files into src/css/, and your webpack config just copies everything in that dir to dist:

❱❱❱ ls dist/css
_fonts.scss
branding.css
ds.css
patternfly-4-cockpit.scss
patternfly-cockpit.scss
patternfly-overrides-variables.scss
patternfly-overrides.scss

This can be avoided by moving the scss files someplace else. I moved them to src/patternfly/.

Also, you don't need the .less building rule, you don't have any less files (and shouldn't start using them now).

But both of these are unrelated -- the main issue is that the webpack now builds a dist/index.css with all the PatternFly stuff, but that is not included in the html. After doing that, it works well.

0001-FIXUP-move-scss-files-out-of-src-css.patch
0002-FIXUP-Re-drop-less-rule.patch
0003-FIXUP-include-generated-webpack-stylesheet.patch

BTW, I'd recommend to not do the "copy src/css/ to dist/css" stuff, but just import your two CSS file in index.es6. Then everything will be together in index.css, and the page doesn't need to load two extra files.

@martinpitt - nice!!! Thank you so much!

And I followed your recommendation about importing the other css files! I appreciate all the tips - I never wrote a web app until this one so I am still a novice :-)

Metadata Update from @mreynolds:
- Issue priority set to: None (was: critical)

4 years ago

Metadata Update from @mreynolds:
- Issue priority set to: critical

4 years ago

Commit 5cc7384 relates to this ticket

Thanks @martinpitt !!! Question, what versions of Fedora are impacted by this deprecation?

Thanks @mreynolds ! This of course depends on when all Cockpit related projects finish the conversion. We track them and help as we can. We currently aim for dropping it in Fedora 34, but depending on porting progress it may already be 33 or 35. But we want to push hard for not shipping it in RHEL 9.

Thanks @mreynolds ! This of course depends on when all Cockpit related projects finish the conversion. We track them and help as we can. We currently aim for dropping it in Fedora 34, but depending on porting progress it may already be 33 or 35. But we want to push hard for not shipping it in RHEL 9.

OK great, thanks for the details, and thanks for the help on this one!

Metadata Update from @mreynolds:
- Issue assigned to mreynolds

4 years ago

Metadata Update from @mreynolds:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

4 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/4240

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: fixed)

4 years ago

Log in to comment on this ticket.

Metadata
Related Pull Requests