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

Updated shift key icon #980

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

devycarol
Copy link
Contributor

This patch addresses #946 by changing the shift icon from up-arrow to Material "shift." This is what the (LineageOS) AOSP keyboard uses now, and is style-concurrent with Holo shift.

It's no longer accent-colored, instead indicating shift state by icon fill. This is in line with Gboard and former Holo, and eliminates accent/background contrast issues.

@devycarol
Copy link
Contributor Author

Already feeling a lot more confident about "did I leave caps lock on?" lol.

@BlackyHawky
Copy link
Contributor

BlackyHawky commented Jul 14, 2024

The current icon is pretty, discreet and sober.
This is just my personal opinion, but the old icon is much prettier.

Does it really need to be changed? 🤔

@devycarol
Copy link
Contributor Author

devycarol commented Jul 14, 2024

Ask the AOSP devs. Granted, I've not fully followed their style either—"caps lock" should have an icon to itself, it shouldn't be the regular "shifted" icon...

More visually symmetrical to the backspace key on the right. With the arrow as the icon, there's an imbalance of icon weight. The chevron isn't used in AOSP anymore, so if you chance upon the AOSP keyboard from time to time it's a visual cue that our app is "behind" design-wise.

To be clear, I do believe "it's modern!!" band-wagoning is generally bad (who decides what's "modern" lol), but this is a case where I think the update is warranted. As I commented in the related issue, the chevron has the appearance of a "Material 1.0" hyper-simplification error, which was eventually patched out.

Also a clearer touch target in "no key borders." I appreciate you chiming in!

@devycarol
Copy link
Contributor Author

devycarol commented Jul 14, 2024

Here's a roster of various themes. From left:

  • Shifted, borders, material darker
  • Non-shifted, no borders, rounded dark
  • Shift-locked, borders, material light.

@BlackyHawky
Copy link
Contributor

For the first time, I disagree, and it seems you're contradicting yourself.

You say: "Who decides what's modern?" Nobody, I agree with you.

But you also say: "that our application is behind in terms of design."

As above, no one can judge.

I repeat: my taste in the choice of icon is purely my personal opinion.

Finally, I think this should be discussed and not imposed (with a poll perhaps) as it changes the visual appearance of the keyboard.

@devycarol
Copy link
Contributor Author

I'm not savvy to this app's design decision process, so I couldn't tell you whether a vote is "how we do things." But I really do agree with you, visual changes shouldn't be taken lightly.

Basically: I like this icon, it's what AOSP uses, and I'll let Helium to decide on it. Perhaps more extensive icon customization could be added down the line—I've had ideas such as "filled" icon theme. This would be cool, but would probably increase APK size. A cool alternative would be a repository of icon themes, similar to dictionaries.

@Helium314
Copy link
Owner

I'm mostly with @BlackyHawky on this. If I were a normal user of this keyboard, I'd probably open an issue after this change. (Also, 2 changes to shift key in a short period seem a little weird).

Perhaps more extensive icon customization could be added down the line

That would be a good thing, as it's a big part of decoupling themes from icon styles. A first idea would be removing the icons from the theme xmls and then do everything via KeyboardIconsSet. But currently I can only work / properly look at code very sporadically, maybe the idea doesn't work at all for reasons I hadn't considered.

would probably increase APK size

With the icons being vector graphics, it's probably only a few kB and will not be noticeable. Especially considering growing size of translations (not compressed in APK) and regular dependency upgrades.

@Helium314
Copy link
Owner

Perhaps more extensive icon customization could be added down the line

That would be a good thing, as it's a big part of decoupling themes from icon styles. A first idea would be removing the icons from the theme xmls and then do everything via KeyboardIconsSet. But currently I can only work / properly look at code very sporadically, maybe the idea doesn't work at all for reasons I hadn't considered.

My current idea, best done in separate commits / PRs:

  1. Put all icons in lists in code instead of using KeyboardIcons style and funnel everything through KeyboardIconsSet. loadIcons would take context instead of keyboardAttrs, get the current style and choose the correct list of icons to load. If really necessary for KeyboardWrapperView or SuggestionStripView, also static (companion object) access to individual icons could be implemented.
  2. Let users choose the icon set independent of theme style, and also each icon from available ones individually (could be implemented separately)
  3. Let users provide icons. This should only happen if it's not too tricky, and maybe there should be some limitations (potential issues with coloring or size)

@BlackyHawky
Copy link
Contributor

@devycarol
The shift icon on the FUTO Keyboard is prettier than the one proposed. You can find them here and here. (Although only the rounded icon is provided and I prefer the current one 😅)

This icon seems larger and more consistent with the delete icon.
I'll let you test it.

@Helium314
Copy link
Owner

Helium314 commented Aug 25, 2024

My current idea, best done in separate commits / PRs:

Part one is now done (edit: and half of part two)

@BlackyHawky FUTO Keyboard has an incompatible license as far as I know, basically they don't allow you to take anything of their stuff.

@BlackyHawky
Copy link
Contributor

I'm too lazy to read their license (long live the vacations!!!).
In any case, I prefer the current icon 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants