Skip to content
This repository has been archived by the owner on Jun 22, 2022. It is now read-only.

Better windows compatibility. #86

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

patricknelson
Copy link
Contributor

@patricknelson patricknelson commented Feb 18, 2017

Setup to help address #14.

Changes (copied from issue):

  • Replaced all icons with twitter emoji: https://github.com/twitter/twemoji
  • On windows, using ctrl + shift in place of command only for the preferences window (,) and quitting (q) and using Windows version of DevTools keyboard shortcut (Control+Shift+I).
  • On Win10, the system tray background can be dark, so removed transparency in the center of the icon (using white).
  • Added yarn.lock lock file, 'cause: shit's fast, yo! 🐱
  • New: Tooltips providing keyword hints. Now you know What keywords to use moving forward if you want to find other emojis (or exactly what portion of what you typed triggered a particular emoji).
  • New: Ability to customize window position. I really like it in the center, not all the way in the corner. The default is still topRight, however!

Here is a demo before/after for the icon (you'll see why I changed it):

Before:
icon before

After:
icon after

Demo:

win10 demo

… compatibility. Also adding title to hint user about keywords and copied. Added yarn since it's hella fast 😺
…orrected systray transparency (on Win it can be dark) and added ability to center window on when loading.
@patricknelson
Copy link
Contributor Author

patricknelson commented Feb 18, 2017

NOTE: Had an issue with the dashed versions not working. Turns out those are ones without any sort of actual unicode but still have an alias, so I'm fixing this too :godmode: :octocat:

@patricknelson
Copy link
Contributor Author

Friend tested on Mac and noticed a bug with the icons, will need an OS-level check to only add the non-transparent version on Windows.

unnamed

@muan
Copy link
Owner

muan commented Mar 11, 2017

Thanks for putting in all these work!

Friend tested on Mac and noticed a bug with the icons, will need an OS-level check to only add the non-transparent version on Windows.

This is probably related to #8. The -Template file name has a purpose.

Replaced all icons with twitter emoji: https://github.com/twitter/twemoji

I'd prefer if this is either an option or flagged for Windows only.

  • New: Tooltips providing keyword hints. Now you know What keywords to use moving forward if you want to find other emojis (or exactly what portion of what you typed triggered a particular emoji).

Nice touch! ❤️

@patricknelson
Copy link
Contributor Author

patricknelson commented Mar 13, 2017

@muan Nope, actually I believe it works fine without the -Template suffix in OSX from what I could tell. Maybe you should check yourself to confirm just in case? I tested this with my aforementioned friend as well and he confirmed it working fine once I put int he OS level check here: 75e8a32. The reason why that OS level check is necessary is because while it looks fine on Mac with transparency, it doesn't look good on Windows since the background can still be dark (see screenshots above).

I'm registering a level of discomfort with standardizing the icons across both OS's, is that correct? If so I can understand since it seems like a drastic change due to the level of native support in the mighty OSX. I like the idea of the app having a consistent appearance across all OS's using standardized icons, as it is already understood that even in Windows why the app itself may not display an emoji, it may still display just fine in Twitter (due to their shims) or in Github (due to Chrome/Chromium/Electron/Webkit still supporting it or via more shims) and it is known that it will also display differently depending on OS.

At least the screen shots/demos displayed would be consistent with user experience regardless of OS, and I think that is a plus. But that's just me and that's my [albeit potentially weak] argument for it.

@patricknelson
Copy link
Contributor Author

patricknelson commented Mar 13, 2017

Also just resolved some merge conflicts against master. Some notes on that:

  1. The transition to CommandOrControl for Linux compatibility was a good one and I've revised my code to instead check to see if the current platform is OSX instead of if it's Windows. This is because Linux and Windows share most of their keyboard shortcuts and Mac is the only one with the "Command" key.
  2. Based on above, now only the icon is checking to see if we're on Windows, so maybe that could be switched as well since again I think only OSX is performing the icon colorization selectively based strictly on transparency so in Linux we could have similar issues as Windows if the OS is styled to have a dark background like on Windows.
  3. Quit shortcut: The code click: function () { app.quit() } was still retained because on Windows, the code selector: 'terminate:' does nothing (doesn't work at all). Please let me know if this doesn't work on OSX. It should, though since I assume this is a standard API for quitting the app 😁

Let me take a look at your focus related question in #85 while I'm still in here.

@patricknelson
Copy link
Contributor Author

@muan I can rebase & resolve conflicts... would you be willing to merge this soon? Any comment? It has sat for a while 😔

@jazzzz
Copy link

jazzzz commented Mar 21, 2018

What is blocking the merge (except the conflicts)?

@patricknelson
Copy link
Contributor Author

That was my main aprehension with resolving the conflicts (which should be easy, haven't checked yet). I didn't want to resolve and have it continue to rot again.

SIDE NOTE: I still use this daily 🎉

@jazzzz
Copy link

jazzzz commented Mar 21, 2018

Also using it daily for nearly one year 🎉

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.

3 participants