Skip to content

webui: introduce themes#1281

Merged
joergsteffens merged 11 commits intobareos:masterfrom
frb121:dev/fbergkemper/master/s5160
Nov 28, 2022
Merged

webui: introduce themes#1281
joergsteffens merged 11 commits intobareos:masterfrom
frb121:dev/fbergkemper/master/s5160

Conversation

@frb121
Copy link
Contributor

@frb121 frb121 commented Oct 17, 2022

Introduces two themes called default and sunflower. The sunflower theme is default for now.

Thank you for contributing to the Bareos Project!

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Is the PR title usable as CHANGELOG entry?
  • Separate commit for CHANGELOG.md ("update CHANGELOG.md"). The PR number is correct.
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems

@frb121 frb121 self-assigned this Oct 17, 2022
@frb121 frb121 changed the title webui: make themes configurable webui: introduce configurable themes Oct 18, 2022
@frb121 frb121 marked this pull request as ready for review October 19, 2022 12:53
@frb121 frb121 requested a review from pstorz October 19, 2022 12:53
@frb121 frb121 requested review from sebastianlederer and removed request for pstorz October 28, 2022 09:02
@frb121 frb121 assigned sebastianlederer and unassigned frb121 Oct 28, 2022
Copy link
Collaborator

@sebastianlederer sebastianlederer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file bareos-logo.png does not seem to be used at all. In my test install, I deleted these files without any noticable effect:

    deleted:    public/img/bareos-logo.png
    deleted:    public/themes/default/img/bareos-logo.png
    deleted:    public/themes/slate/img/bareos-logo.png
    deleted:    public/themes/sunflower/img/bareos-logo.png

So it is likely that these files can be removed entirely.

The other redundant PNG files and font files are probably unavoidable without much more changes in the PHP code and in the bootstrap CSS.

@frb121
Copy link
Contributor Author

frb121 commented Nov 14, 2022

The file bareos-logo.png does not seem to be used at all. In my test install, I deleted these files without any noticable effect:

    deleted:    public/img/bareos-logo.png
    deleted:    public/themes/default/img/bareos-logo.png
    deleted:    public/themes/slate/img/bareos-logo.png
    deleted:    public/themes/sunflower/img/bareos-logo.png

So it is likely that these files can be removed entirely.

The other redundant PNG files and font files are probably unavoidable without much more changes in the PHP code and in the bootstrap CSS.

I'd really like to avoid removing original source image material from source tree even if it's not actively used in the application currently.

If a theme doesn't and probably never will use the mentioned one above, we could remove it, I agree.

But let's keep and not touch originals in the default image folder (public/img/).

Our README.md for example references an image located in here as well,
another reason I kept the folder and images for now.

If we want to reduce application size, build and packaging should handle this.

We could move the img folder one directory up to place it in webui root folder and have the source image material available there, I guess. That might already exclude it from being packaged, but I have to check that out.

In addition this change would require to adjust the image reference in our https://github.com/bareos/bareos/blob/master/README.md .

@frb121
Copy link
Contributor Author

frb121 commented Nov 15, 2022

Removed unused image material from themes except one source image for reference in the default theme folder I'd like to keep for now.

Also adjusted the Bareos logo include in our README.md in a separate commit due to the recent changes.

@frb121
Copy link
Contributor Author

frb121 commented Nov 18, 2022

Note: This PR requires #1313 to be merged first. Afterwards we need to rebase this one.

@frb121 frb121 changed the title webui: introduce configurable themes webui: introduce themes Nov 22, 2022
@frb121
Copy link
Contributor Author

frb121 commented Nov 22, 2022

Removed the experimental slate theme and reduced the left themes to its minimum.

@frb121
Copy link
Contributor Author

frb121 commented Nov 22, 2022

Updated PR title and description.

@frb121
Copy link
Contributor Author

frb121 commented Nov 22, 2022

CHANGELOG.md needs to be adjusted.

@frb121
Copy link
Contributor Author

frb121 commented Nov 22, 2022

We should also squash commits before merge for a clean commit history. I kept it due to the review process for now as it is.

@frb121 frb121 requested a review from joergsteffens November 22, 2022 16:01
@frb121 frb121 assigned joergsteffens and unassigned frb121 Nov 22, 2022
@joergsteffens
Copy link
Member

It might be good idea, to refer to the source of the sunflower picture, at least in the git commit message.

@joergsteffens joergsteffens dismissed sebastianlederer’s stale review November 28, 2022 15:41

Suggestions got applied.

@joergsteffens joergsteffens merged commit e1e81d4 into bareos:master Nov 28, 2022
@frb121 frb121 deleted the dev/fbergkemper/master/s5160 branch April 21, 2023 10:47
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

Successfully merging this pull request may close these issues.

3 participants