probberechts / hexo-theme-cactus

:cactus: A responsive, clean and simple theme for Hexo.
https://probberechts.github.io/hexo-theme-cactus/
MIT License
3.2k stars 784 forks source link

Add logo grayout option #214

Closed monkeyWzr closed 4 years ago

monkeyWzr commented 4 years ago

Hey there @probberechts Thanks for your great theme!

I tried to add an option to turn on/off the logo grayout effect (#210). Since it seems the current merge-config script is using shadow merge, I added a logo-grayout option out of logo configs. I'm wondering if it's ok.

I'll be appreciated if you can have a look at my pr. Thank you!

probberechts commented 4 years ago

I would use logo.grayout instead of logo-grayout as the option name to be consistent with the other settings. Otherwise, this seems perfect.

monkeyWzr commented 4 years ago

I would use logo.grayout instead of logo-grayout as the option name to be consistent with the other settings. Otherwise, this seems perfect.

Thanks for your advice! But I'm afraid that won't work if we use the name 'logo.grayout' because after merging, the config object will be like:

hexo.theme.config = {
    ...
    logo: {
        enabled: true,
        ...
    },
    'logo.grayout': false
}

And hexo-config("logo.grayout") will just try to find grayout property in logo. https://github.com/hexojs/hexo-renderer-stylus/blob/1003057634a500badd6aa1063dbcb376409f64b2/lib/renderer.js#L5-L28

Or did you mean I should change the option name to underscore version logo_grayout instead?

probberechts commented 4 years ago

I would do it like this:

logo:
  enabled: true
  width: 50
  height: 50
  url: /images/logo.png
  gravatar: false
  grayout: true