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

lxqt.pcmanfm-qt: fix default wallpaper #87623

Merged
merged 1 commit into from Jun 21, 2020
Merged

Conversation

@wamserma
Copy link
Contributor

wamserma commented May 11, 2020

Motivation for this change

#70780

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@wamserma wamserma mentioned this pull request May 11, 2020
1 of 10 tasks complete
@ofborg ofborg bot requested a review from romildo May 12, 2020
@romildo
Copy link
Contributor

romildo commented May 13, 2020

This change sets the wallpaper to one with a NixOS brand.

What other distributions have done is make a new branding theme and set it as the default. Maybe we should follow them as soon as the panel/theme issue is fixed.

I am not sure about committing this change and would like to see other opinions.
cc @worldofpeace @jtojnar

@wamserma
Copy link
Contributor Author

wamserma commented May 13, 2020

Yes, setting a branded wallpaper is an opinionated decision.
The current default points at a non-existent wallpaper, sets a black background. Together with the default panel background, this is a bad UX.
Fixing the wallpaper is easy and a quick win, while the panel/theme issue linger around for some more time, I suspect.
Regarding the branded wallpaper, I was inspired by #86163 but I favour the blue wallpaper.

@wamserma
Copy link
Contributor Author

wamserma commented Jun 15, 2020

@jtojnar
Copy link
Contributor

jtojnar commented Jun 15, 2020

I am not sure apps should depend on system theme. Is there a way to set the background in a NixOS module rather than pcmanfm?

@wamserma
Copy link
Contributor Author

wamserma commented Jun 16, 2020

I updated the PR to just fix the path instead of using NixOS-artwork.
The main problem was that the template config is copied to user's home at first login, and without this change the Wallpaper would be set to a file in the Nix Store that is eventually GC'd (due to the hardcoded path). Now using the proper symlink from /run/current-system/sw....

@jtojnar Ok for you?

@wamserma
Copy link
Contributor Author

wamserma commented Jun 19, 2020

@jtojnar
Copy link
Contributor

jtojnar commented Jun 19, 2020

Looks like that should work, at least in LxQt.

The path is linked:

# Link some extra directories in /run/current-system/software/share
environment.pathsToLink = [ "/share" ];

And the package is installed unconditionally

I am not a fan of using this nexus but I guess it is fine until #47173 is resolved.

@wamserma wamserma force-pushed the wamserma:lxqt-fix-wallpaper branch from 7cc50fc to 8e5d419 Jun 19, 2020
@wamserma
Copy link
Contributor Author

wamserma commented Jun 19, 2020

This is currently imho the best (and only without further indirection and hackery) way to have a stable path that always points to the currently active lxqt-themes package.

Also fixed a missing slash in this path.

pkgs/desktops/lxqt/pcmanfm-qt/default.nix Outdated Show resolved Hide resolved
@wamserma wamserma requested a review from romildo Jun 20, 2020
Copy link
Contributor

romildo left a comment

There are 3 commits. Reduce them to just one. Then I think it will be ready for merging.

@wamserma wamserma force-pushed the wamserma:lxqt-fix-wallpaper branch from 1cac8f9 to f805871 Jun 20, 2020
@wamserma
Copy link
Contributor Author

wamserma commented Jun 20, 2020

There are 3 commits. Reduce them to just one. Then I think it will be ready for merging.

Yes. That was meant for easier review. Should be good for merging now.

@wamserma wamserma requested a review from romildo Jun 20, 2020
@romildo romildo merged commit 6a12c20 into NixOS:master Jun 21, 2020
16 checks passed
16 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f805871"; rev="f805871a51c6b9ef50d2860c8c08b57d293f0740"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f805871"; rev="f805871a51c6b9ef50d2860c8c08b57d293f0740"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f805871"; rev="f805871a51c6b9ef50d2860c8c08b57d293f0740"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f805871"; rev="f805871a51c6b9ef50d2860c8c08b57d293f0740"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f805871"; rev="f805871a51c6b9ef50d2860c8c08b57d293f0740"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f805871"; rev="f805871a51c6b9ef50d2860c8c08b57d293f0740"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f805871"; rev="f805871a51c6b9ef50d2860c8c08b57d293f0740"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
lxqt.pcmanfm-qt, lxqt.pcmanfm-qt.passthru.tests on aarch64-linux Success
Details
lxqt.pcmanfm-qt, lxqt.pcmanfm-qt.passthru.tests on x86_64-linux Success
Details
@wamserma wamserma deleted the wamserma:lxqt-fix-wallpaper branch Jun 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.