Skip to content
This repository was archived by the owner on Jun 29, 2019. It is now read-only.

Conversation

@dannya
Copy link
Member

@dannya dannya commented May 18, 2018

Implement extensive theme support!

Comes bundled with default Light and Dark themes.
There's scope for further improvements (I have some more ideas!) and tweaks to the theme interpretation, but it worked well with the 10 random 3rd party themes I tested this with.

Additional 3rd party theme files (see below for the formats supported) simply need to be dropped into the user preferences directory, eg. C:\Users\danny\AppData\Local\hain-user\themes where they will be scanned on app startup.

There is support for rgba() colours (ie. transparency) in themes, with a new user config setting to enable transparency, but this exposes an Electron bug around windows flickering on show, so it is off by default.

This has been implemented using the Alfred JSON theme format internally, with a best effort attempt to automatically convert these theme types into this format for use:

  • Alfred JSON
  • Alfred XML (however some use serialized NSColor objects that are not parsable, so if this is detected the file is not loaded)
  • Themer JS export files

Other text-based theme formats are easy to support in future as formats are modular.

Here is the internal Alfred-compatible JSON theme format representing the default "Hain Light" theme:

{
name: 'Hain Light',
credit: 'dannya',

result: {
textSpacing: 6, // not implemented
subtext: {
size: 13,
colorSelected: '#5E5E5E',
font: '"Roboto", sans-serif', // partially-implemented
color: '#757575'
},
shortcut: {
size: 16, // not implemented
colorSelected: '', // not implemented
font: '', // not implemented
color: '' // not implemented
},
backgroundSelected: '#DDDDDD',
text: {
size: 18,
colorSelected: '#272727',
font: '"Roboto", sans-serif', // partially-implemented
color: '#212121'
},
iconPaddingHorizontal: 6, // not implemented
paddingVertical: 6, // not implemented
iconSize: 40 // not implemented
},
search: {
paddingVertical: 8,
background: '#ffffff',
spacing: 6,
text: {
size: 22,
colorSelected: '', // not implemented
font: '"Roboto", sans-serif', // partially-implemented
color: '#000000'
},
backgroundSelected: '' // not implemented
},
window: {
color: '#ffffff',
paddingHorizontal: 10, // not implemented
width: 560,
height: 544,
borderPadding: 0, // not implemented
borderColor: '', // not implemented
blur: 15, // not implemented
roundness: 2, // not implemented
paddingVertical: 10 // not implemented
},
separator: {
color: '#00BCD4',
thickness: 0 // not implemented
},
scrollbar: {
color: '#CCCCCC',
thickness: 10
}
}

dannya added 16 commits May 13, 2018 15:41
npm WARN deprecated lodash.isarray@4.0.0: This package is deprecated. Use Array.isArray.
… more config pages (themes) later.

* Improve layout and wording of main app config UI.
* Position loading spinner in center of preferences window.
Comes bundled with default Light and Dark themes.
There's scope for further improvements (I have some more ideas!) and tweaks to the theme interpretation, but it worked well with the 10 random 3rd party themes I tested this with.

Additional 3rd party theme files (see below for the formats supported) simply need to be dropped into the user preferences directory, eg. C:\Users\danny\AppData\Local\hain-user\themes where they will be scanned on app startup.

There is support for rgba() colours (ie. transparency) in themes, with a new user config setting to enable transparency, but this exposes an Electron bug around windows flickering on show, so it is off by default.

This has been implemented using the Alfred JSON theme format internally, with a best effort attempt to automatically convert these theme types into this format for use:
* Alfred JSON
* Alfred XML (however some use serialized NSColor objects that are not parsable, so if this is detected the file is not loaded)
* Themer JS export files

Other text-based theme formats are easy to support in future as formats are modular.

Here is the internal Alfred-compatible JSON theme format representing the default "Hain Light" theme:

{
  name: 'Hain Light',
  credit: 'dannya',

  result: {
    textSpacing: 6,     // not implemented
    subtext: {
      size: 13,
      colorSelected: '#5E5E5E',
      font: '"Roboto", sans-serif',     // partially-implemented
      color: '#757575'
    },
    shortcut: {
      size: 16,     // not implemented
      colorSelected: '',     // not implemented
      font: '',     // not implemented
      color: ''     // not implemented
    },
    backgroundSelected: '#DDDDDD',
    text: {
      size: 18,
      colorSelected: '#272727',
      font: '"Roboto", sans-serif',     // partially-implemented
      color: '#212121'
    },
    iconPaddingHorizontal: 6,     // not implemented
    paddingVertical: 6,     // not implemented
    iconSize: 40     // not implemented
  },
  search: {
    paddingVertical: 8,
    background: '#ffffff',
    spacing: 6,
    text: {
      size: 22,
      colorSelected: '',     // not implemented
      font: '"Roboto", sans-serif',     // partially-implemented
      color: '#000000'
    },
    backgroundSelected: ''     // not implemented
  },
  window: {
    color: '#ffffff',
    paddingHorizontal: 10,     // not implemented
    width: 560,
    height: 544,
    borderPadding: 0,     // not implemented
    borderColor: '',     // not implemented
    blur: 15,     // not implemented
    roundness: 2,     // not implemented
    paddingVertical: 10     // not implemented
  },
  separator: {
    color: '#00BCD4',
    thickness: 0     // not implemented
  },
  scrollbar: {
    color: '#CCCCCC',
    thickness: 10
  }
}
Copy link
Collaborator

@appetizermonster appetizermonster left a comment

Choose a reason for hiding this comment

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

Thank you for this amazing work!!
I've just looked the code you work. (I didn't run it yet)
It looks very clean and neat! Great!
and I just request very minor changes 😃

@appetizermonster
Copy link
Collaborator

I've tested it on my computer, and it looks great but this PR breaks some important features 😢

  1. vibrancy (only on macOS)
  2. preview (you can see it when you use hain-plugin-naverdictionary https://github.com/appetizermonster/hain-plugin-naverdictionary)
  • before:
    image

  • after:
    image

@dannya
Copy link
Member Author

dannya commented May 20, 2018

@appetizermonster no worries, I'll look into these issues and commit an update soon.

dannya added 5 commits May 27, 2018 15:48
…tting page, as we will need to treat them as linked settings for some scenarios
…r modification for correct cross-platform transparency behaviour
… setting asking if they want to restart the app (required to effect this setting change), or revert the setting change
dannya added 6 commits May 28, 2018 13:27
…t (ie. on by default on MacOS, off by default on Windows)
* Autodetect dark/light variant value direcly from the theme-defined window color
* Restore wider main window default size to for allow preview panel
@dannya
Copy link
Member Author

dannya commented May 28, 2018

@appetizermonster I've refactored this to address the issues you raised, please re-review.

@appetizermonster
Copy link
Collaborator

Thanks!!! I'll review tonight (in KST) or sooner

Copy link
Collaborator

@appetizermonster appetizermonster left a comment

Choose a reason for hiding this comment

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

Code really looks good to me!! 👍
and the theme feature is really good!!!
we only have some problems with vibrancy yet.

I think you can merge this PR at this time, and then I can fix the rest of the problems later because I'm using mac for now.

@appetizermonster
Copy link
Collaborator

FYI, I attach screenshots on my mac with the current build

current build (no vibrancy, but backgroundSelected color is okay)

screen shot 2018-05-30 at 11 53 12 pm

removing background color (vibrancy, but backgroundSelected color isn't visible)

screen shot 2018-05-30 at 11 55 57 pm

removing background color and set backgroundSelected to rgba format

screen shot 2018-05-30 at 11 56 46 pm

@dannya
Copy link
Member Author

dannya commented May 30, 2018

@appetizermonster thanks, this is helpful.

What change did you do to remove the background color for your testing?

@appetizermonster
Copy link
Collaborator

I've just commented out these lines

      this.browserWindow.webContents.insertCSS(
        `html { background: ${windowColor} !important; }`
      );

@dannya
Copy link
Member Author

dannya commented Jun 4, 2018

@appetizermonster please see my latest commits to this PR, this is the output I see on Mac which looks acceptable to me:

hain_light_mac
hain_dark_mac

@cpriest
Copy link
Member

cpriest commented Jun 5, 2018

Just thought I'd chime in, this is really great, thank you for the contribution! Hadn't really had time to keep up with the conversation until this morning.

@appetizermonster
Copy link
Collaborator

AWESOME! I've read the recent changes. looks good to me 😄

@dannya dannya merged commit 3f4f5c0 into develop Jun 5, 2018
@dannya dannya deleted the feature/themes branch June 5, 2018 19:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants