end-4 / dots-hyprland

i hate minimalism so...
https://end-4.github.io/dots-hyprland-wiki/en/
GNU General Public License v3.0
4.59k stars 322 forks source link

[Issue] config_overviewOnly.js nits #418

Open jennydaman opened 7 months ago

jennydaman commented 7 months ago

Hi there, I really like the overview feature and got it working on NixOS. However I encountered a few nits:

https://github.com/end-4/dots-hyprland/blob/f173fbb7ad44b8800aecf5ee3f521edf4b6fd511/.config/ags/config.js#L36-L37

These lines of code are not present in config_overviewOnly.js, so I had to manually touch _musicwal.scss _musicmaterial.scss.

https://github.com/end-4/dots-hyprland/blob/f173fbb7ad44b8800aecf5ee3f521edf4b6fd511/.config/ags/config_overviewOnly.js#L1-L2

I suggest adding material-symbols and xorg-xrandr* to the list of dependencies here.

https://github.com/end-4/dots-hyprland/blob/f173fbb7ad44b8800aecf5ee3f521edf4b6fd511/.config/ags/variables.js#L18-L20

*Regarding the xrandr dependency: would it be better to get this information at runtime from hyprctl monitors instead? (which will also be better in situations where monitors are plugged in/out dynamically, e.g. an external monitor is attached to a laptop) LMK if you'd accept a PR for this!

end-4 commented 7 months ago

LMK if you'd accept a PR for this!

sure

jennydaman commented 7 months ago

@end-4 I'm new to this repository (and also ags). Do you have a doc or anything similar for coding style?

In overview_hyprland.js I see Promises being chained with .then(...)... Replacing the SCREEN_WIDTH and SCREEN_HEIGHT global variables with a call to execAsAsync will exacerbate the indentation pyramid of doom. If you're OK with that I'll do it that way to achieve a minimal patchset. Alternatively, I could group together execAsAsync with Promises.all and refactor some nested arrow functions into top-level named functions.

Second, would you accept a PR which only removes the xrandr dependency from config_overviewOnly.js, or would you want me to fix it everywhere? Since I am on NixOS and using only config_overviewOnly.js, if I were to work on the other uses of xrandr in hypr/hyprland/keybinds.conf and ags/scripts/color_generation/switchwall.sh, that will be much more work for me and I'd have to set up an Arch Linux VM.

end-4 commented 7 months ago

Promise chains

About xrandr