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

11 new themes based on material color #16

Closed
wants to merge 11 commits into from
Closed

11 new themes based on material color #16

wants to merge 11 commits into from

Conversation

BlackyHawky
Copy link
Contributor

@BlackyHawky BlackyHawky commented Jul 14, 2023

I created 11 predefined themes based on material colors.
These themes work with or without key borders.
Thanks to @odmfl, the color of the navigation bar follows the color of the keyboard background (everything works correctly, except in the automatic theme). #737

These commits are tested on Samsung Tablet with Android 13 and on Huawei phone with Android 10.

Screenshots of some themes below.

@BlackyHawky BlackyHawky changed the title Update Emojis to version 15 Update Emojis to version 15 + 11 new themes + emoji usage frequency Jul 14, 2023
@odmfl
Copy link

odmfl commented Jul 15, 2023

very nice, how does it behave with the tablet? Is typing optimized there too? Can you send some more screenshots from the tablet?

@RHJihan
Copy link
Contributor

RHJihan commented Jul 15, 2023

The navigation bar color is problematic with the Holo (Legacy) theme.
The navigation bar color is not following the preference Color navigation bar. It is always matching with the theme of the keyboard. I guess removing the preference Color navigation bar will be better.

While Key borders is turned off, Keyboard background of user-defined theme not working, it is only affecting the navigation color.

Android 13
One UI 5.1

Holo (Legacy) Color navigation bar toggle not working

@odmfl
Copy link

odmfl commented Jul 15, 2023

@BlackyHawky You did a great job, thank you.
Speaking of emojis, can you see if you can improve emoji search in my PR #2 ?
Currently it works but only with English, but it needs to be revised, can you take a look at it?

@BlackyHawky
Copy link
Contributor Author

very nice, how does it behave with the tablet? Is typing optimized there too? Can you send some more screenshots from the tablet?

@odmfl Thanks a lot for your feedback.
It's fully optimized for my Galaxy Tab A8 tablet.
I guess it's good for those who use a tablet with a similar screen even those with a smaller tablet screen.
Screenshots of tablet below.

Screenshot_Brown_Tablet
Screenshot_Green_Tablet
Screenshot_Red_Tablet

@odmfl
Copy link

odmfl commented Jul 15, 2023

@BlackyHawky really excellent, finally a good user experience even on the tablet. I tested a test apk with your commits, your themes work fine with key borders, without borders they should be changed. If you can you could also make some modified dark theme. In addition to custom themes, two “material you” themes (light and dark) compatible with A12 and A13 devices would be great

@odmfl
Copy link

odmfl commented Jul 15, 2023

@BlackyHawky as far as the emojis frequency are concerned, in my opinion that commit is not needed, the behavior is good the default one, only the emoji search needs to be improved by expanding the search to other languages

@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Jul 15, 2023

@RHJihan : Now, with my last commit, color of navigation bar follow background color of holo, light and dark theme properly.

About the preference Color navigation bar, it seems to me that it never worked properly.
Maybe @Helium314 will do an update.

Indeed, I confirm the main bug when selecting the keyboard background color when User-defined is choosen: when you validate a color and then change it, it doesn't update but the navigation bar does.
To change the color, either close the application or toggle without key borders.

Moreover, after choosing User-defined as a variant, if you then choose a predefined theme, the color of the emoji + language and delete icons remains white.
Force close the application corrects this bug too.

@RHJihan
Copy link
Contributor

RHJihan commented Jul 15, 2023

@BlackyHawky, the Navigation bar color is still problematic when Auto day/night mode is enabled and the device display is in Dark mode.

@BlackyHawky
Copy link
Contributor Author

@BlackyHawky as far as the emojis frequency are concerned, in my opinion that commit is not needed, the behavior is good the default one, only the emoji search needs to be improved by expanding the search to other languages

@odmfl : I suspected it. So I'm thinking of removing this change. Too bad because I found this function very cool.
I will do it tomorrow or last week.

@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Jul 15, 2023

@BlackyHawky, the Navigation bar color is still problematic when Auto day/night mode is enabled and the device display is in Dark mode.

@RHJihan : as mentionned by @odmfl in this PR everything works correctly, except in the automatic theme.
Since I'm not a developer but self-taught, I'm having trouble finding the solution.

@odmfl
Copy link

odmfl commented Jul 15, 2023

The "follow the system" theme needs to be redone. There is probably something wrong there. Also I am self taught I am not a professional developer. Maybe redoing that “follow the system” theme might fix itself. In practice, "follow the system" and "follow system border", like this Evervolv/android_packages_inputmethods_LatinIME@e4d3e02

@RHJihan
Copy link
Contributor

RHJihan commented Jul 16, 2023

@BlackyHawky, seems like Recent Emoji is not working.

@BlackyHawky BlackyHawky changed the title Update Emojis to version 15 + 11 new themes + emoji usage frequency Update Emojis to version 15 + 11 new themes Jul 16, 2023
@BlackyHawky
Copy link
Contributor Author

@BlackyHawky as far as the emojis frequency are concerned, in my opinion that commit is not needed, the behavior is good the default one, only the emoji search needs to be improved by expanding the search to other languages

@odmfl : I suspected it. So I'm thinking of removing this change. Too bad because I found this function very cool. I will do it tomorrow or last week.

@odmfl : As discussed yesterday, I've removed the "Added emoji usage frequency support" feature.
@RHJihan : Now, you won't have any more problems with Recent Emoji.

@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Jul 16, 2023

For a better experience with new themes, take into account the last commit "Adjustment of some values for keyboard rows + split number row.

  • PR #630 corrected ;
  • With these modifications, you should have the same layout as screenshots above ;
  • Symbols are better positioned because the keys of my themes are a little more rounded (doesn't affect the original themes that much) ;
  • Number Row is separated when 'Enable split keyboard' is toggle on ;
  • Tip : for AZERTY / QWERTY / QWERTZ (I didn't test with others), when Number Row is enabled, resize the keyboard to 80%.
    If Number Row is disabled, resize the keyboard to 100%.
    This allows the height of the keyboard to almost match the height of the emojis and clipboard page.
  • Missing french translations added

I'm waiting your feedback.

@RHJihan
Copy link
Contributor

RHJihan commented Jul 16, 2023

Number Row is not separated from my build when Enable split keyboard is toggled on.
AVD: Nexus 10 API 28 2560x1600xhdpi

@BlackyHawky
Copy link
Contributor Author

Number Row is not separated from my build when Enable split keyboard is toggled on. AVD: Nexus 10 API 28 2560x1600xhdpi

@RHJihan : I'll look that ; I'm suprised cause your build seems to be correct.

For AZERTY + QWERTY + QWERTZ with french, english & german language everything works fine.

@BlackyHawky
Copy link
Contributor Author

Number Row is not separated from my build when Enable split keyboard is toggled on. AVD: Nexus 10 API 28 2560x1600xhdpi

@RHJihan : I'll look that ; I'm suprised cause your build seems to be correct.

For AZERTY + QWERTY + QWERTZ with french, english & german language everything works fine.

@RHJihan : I implemented your #18 in my builds and everything works fine in portrait and landscape mode.
Maybe can you try to force close the app and clear data + cache and try again.

Screenshot_Bengali

@RHJihan
Copy link
Contributor

RHJihan commented Jul 16, 2023

yes! Everything works fine now.

Though now I have to change the keyWidth for bn_BD to match with the other layouts.

@RHJihan
Copy link
Contributor

RHJihan commented Jul 16, 2023

@BlackyHawky BlackyHawky changed the title Update Emojis to version 15 + 11 new themes 11 new themes based on material color Jul 17, 2023
@RHJihan
Copy link
Contributor

RHJihan commented Jul 19, 2023

Will less spreading shadow improve the appearance?

Untitled-1

@BlackyHawky
Copy link
Contributor Author

Will less spreading shadow improve the appearance?

For information : this is done intentionally in order to have a real difference between the popups of the letters and the popups of the hints/suggestions panel.

I will not be objective if I give my opinion since I created these themes so give me feedback if it is really necessary to modify the shadows.

@RHJihan
Copy link
Contributor

RHJihan commented Jul 19, 2023

That much shadow seems not eye soothing to me. The popup shadow on the Light theme and on Gboard is subtle too.

@BlackyHawky
Copy link
Contributor Author

Light theme isn't modified so this is normal that shadow is subtle or rather non-existent, because it's a shadow simulation.
About Gboard, I assumed that if people wanted Gboard's interface, they could simply download Gbord. For me, Gboard should only be a source of inspiration.

@RHJihan
Copy link
Contributor

RHJihan commented Jul 19, 2023

A big and rather noticeable shadow doesn't quite sit well with me aesthetically. My suggestion is to go for a more subtle and smaller shadow instead.

Just to clarify, I'm not suggesting that we directly copy Gboard, but rather, I'd like to emphasize the beauty of using a more understated shadow. It can create a sleek and polished look without being overly distracting. Gboard is a good example for it.

Another thing— is the width of popup is relatively small for a single character? existing shadow looks good on popup with multiple characters, but on with a single character.

@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Jul 19, 2023

I'm disappointed that this little detail doesn't suit you aesthetically.

What you describe only appears if you have the "Popup on keypress" setting enabled and only for keys with a single character.

Shadows are handled by "morekey_shadow_*" files.
When I created these themes, I tried to reduce the shadow on popups where there is only one character, but this significantly reduced the shadows of the suggestion panel popups and others. Trust me, I've done a lot of testing. So I left that as is.

If there is another and better solution to manage the shadows of these popups, I'm interested because I don't have sufficient knowledge.

About the width of the popup, this is not a bug; this only appears if the "Popup on keypress" setting is enabled again.
You can test with the Light theme which has not been modified and you will see the same thing. This is therefore not due to the proposed themes but to the original code.

@Helium314
Copy link
Owner

This is a huge PR (300 files), and it seems to change every day or so, which makes it really hard to review. Additionally it seems to contain changes unrelated to the addition of themes.
These other changes should not be in here, please open separate PRs for them. Splitting the PR into independent parts is much easier for reviewing and discussion.

Also, why is this PR completely ignoring the possiblity of setting theme colors using what is currently in the user theme?

@BlackyHawky
Copy link
Contributor Author

Indeed, I was very hesitant to post this PR because I knew that the number of files would be problematic.
If there are so many files it's because I based myself on the light theme which is defined by about 20-25 files so 11 more themes...
Moreover, the themes are independent of each other.

Also, I unashamedly admit that it was my lack of knowledge that caused other commits to slip into this PR and I apologize for that.

I confirm that the user always has the possibility of using the colors of his choice; this PR was posted after your commit cdabf65.

Currently, no changes are expected.

@Helium314
Copy link
Owner

Also, I unashamedly admit that it was my lack of knowledge that caused other commits to slip into this PR and I apologize for that.

Ah, I see you started this PR from the main branch... Not sure how to best disentangle this.
Still I would like to have the changes not related to the themes in separate pull requests. This simplifies my work, and some parts can probably be merged quite soon (e.g. the translations)

I confirm that the user always has the possibility of using the colors of his choice; this PR was posted after your commit cdabf65.

Sorry, it seems my question was unclear: I would like to know why the new themes do not make use of the possibility to set custom colors, as it's done here:
https://github.com/Helium314/openboard/blob/bbfebcc50a72bf1202a2ef29644933b6e8eda7a0/app/src/main/java/org/dslul/openboard/inputmethod/latin/settings/SettingsValues.java#L267-L295
Would it be enough to set the colors to your theme colors instead of reading user colors from the settings? Only thing that would need to be added/changed is a color for the "special" keys (shift, delete, ...).
Or is something else missing?

@BlackyHawky
Copy link
Contributor Author

Ah, I see you started this PR from the main branch... Not sure how to best disentangle this.

I know the solution : close this PR, create a new branch "in my repository" (I forgot to do this at the beginning) and create a new PR from this branch.

Still I would like to have the changes not related to the themes in separate pull requests. This simplifies my work, and some parts can probably be merged quite soon (e.g. the translations)

Of course ; easy to do.

Would it be enough to set the colors to your theme colors instead of reading user colors from the settings? Only thing that would need to be added/changed is a color for the "special" keys (shift, delete, ...).
Or is something else missing?

I understand and think you are right. This would avoid adding and/or modifying 300 files. We need to improve this part of code instead.

https://github.com/Helium314/openboard/blob/bbfebcc50a72bf1202a2ef29644933b6e8eda7a0/app/src/main/java/org/dslul/openboard/inputmethod/latin/settings/SettingsValues.java#L267-L295

I don't see any harm in you closing this PR.

@Helium314
Copy link
Owner

Helium314 commented Jul 20, 2023

I adjusted the theming a bit, now the colors are set in Settings.getCustomColors, and you can choose separate colors for function keys and space bar.
(Navbar coloring is currently completely broken if you don't set a custom color, but this should not be relevant for you. I'll fix it in a few days)

I converted the PR to a draft, because I think keeping it open as long as not everything is implemented could serve as a reminder. Feel free to close it if you think otherwise.

@Helium314 Helium314 marked this pull request as draft July 20, 2023 10:52
@BlackyHawky BlackyHawky closed this by deleting the head repository Jul 20, 2023
@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Jul 21, 2023

### For information ###

This PR was not closed because it was not functional but because Helium314 is working on improving themes whose colors are defined by the user.
This avoids having nearly 300 files to add and/or modify.

However, you can always take inspiration from these predefined themes that I leave available in my Repository.
This commit contains only the necessary files for the predefined themes ; color of navigation bar works too.

Thanks to Hélium314 for his very good work on this topic.

@odmfl
Copy link

odmfl commented Jul 22, 2023

### For information ###

This PR was not closed because it was not functional but because Helium314 is working on improving themes whose colors are defined by the user. This avoids having nearly 300 files to add and/or modify.

However, you can always take inspiration from these predefined themes that I leave available in my Repository. This commit contains only the necessary files for the predefined themes and nothing else.

Thanks to Hélium314 for his very good work on this topic.

thanks for these themes, I wanted to tell you that with the changes made to the main branch, now in your themes the navbar is broken, from different colors

@Helium314
Copy link
Owner

The current state of custom themes:

  • colors are provided using Colors
  • colors are (mostly) applied via BlendModeColorFilters in BlendMode.MODULATE on top of the light theme
    • this means that colors can't be lighter than the underlying light theme
  • bug: action and comma key popups not colored

Possible extensions:

  • base the custom themes on a purely white theme to allow full color range
  • new themes could be added using the custom theming system, i.e. pre-defined Colors for each theme
    • @BlackyHawky are you still interested in doing this, but using the new system? Maybe just convert what you created here? The themes definitely look good!
  • have adjustable / user-selectable shadows? I did not investigate at all how this works, just adding it because it came up in this discussion
  • existing themes could be converted to use Colors
    • this needs to take into account day/night themes
    • if this is done, auto-mode could be made more flexible to switch between any 2 themes (or Colors could contain day and night colors for each theme)
  • allow user to provide or select images (drawables) for background and keys. I'm like 80% sure this will work. The colors will also be adjusted by theme colors, as they are applied via color filters. Though we might want to allow selecting the alpha channel for key background color then
  • have a preview, so the effect of changed color can be seen immediately (may require adding some sort fake keyboard, possibly considerable work)

@RHJihan
Copy link
Contributor

RHJihan commented Jul 22, 2023

@BlackyHawky, please consider opening a PR for the Adjustment of some values for keyboard rows + split number row. This was a good commit.

@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Jul 22, 2023

thanks for these themes, I wanted to tell you that with the changes made to the main branch, now in your themes the navbar is broken, from different colors

@odmfl : you're welcome.
I just created a branch in my repository which corrects the color of the navigation bar for those who are interested: Theme_&_Navbar_color_working Edit 10/08/2023: Branch removed as theme enhancements allow custom colors to be defined without the need for additional files.

  • @BlackyHawky are you still interested in doing this, but using the new system? Maybe just convert what you created here? The themes definitely look good!

@Helium314 : Impressive work done on this subject. 💪 Thanks a lot!
Of course I'm interested. I'll try to work on it this weekend. Either way, I'll keep you all informed very soon.

@BlackyHawky, please consider opening a PR for the Adjustment of some values for keyboard rows + split number row. This was a good commit.

@RHJihan : #27 created for Split number row. 👍
For the Adjustments of some values, I will work on it again to really modify the important ones.

@Helium314
Copy link
Owner

Of course I'm interested. I'll try to work on it this weekend. Either way, I'll keep you all informed very soon.

Great, thanks! No need for soon though.

I really think the "base the custom themes on a purely white theme to allow full color range" part needs to be done first, because doing this after implementing your themes will change the resulting colors.
So we would need (a border and a no border) theme where all keys/buttons have a white background, and a bit darker when pressed. This theme should be active whenever the custom theme is used (currently it's set to use the light theme). Can you do this? I think it should not be much different than adding other themes as you did in this PR.

@BlackyHawky BlackyHawky mentioned this pull request Jul 22, 2023
@Helium314
Copy link
Owner

@BlackyHawky did you already start working on this? If not, I will do the "(a border and a no border) theme where all keys/buttons have a white background, and a bit darker when pressed" part.

@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Jul 25, 2023

@Helium314 I finished no border base theme. (I work nights at the moment so I take a little time.)

For information: before making a new PR, I will make commits in my repository for you to analyze all of this. Is it okay for you?

No border base theme here : f8694bd

  • KeyboardTheme.java and AppearanceSettings.kt are modified simply to test the base theme. (All is white so)

  • Enter key is entirely white because the key and arrow are white. It's not a bug.

  • No shadows are included ; we'll see later.

  • About thirty files will be analyzed with both theme.

@Helium314
Copy link
Owner

@Helium314 I finished no border base theme. (I work nights at the moment so I take a little time.)

No problem, take your time. Meanwhile I'll adjust some custom-color related things, mostly in KeyboardView.java. I hope it does not interfere with your work.

For information: before making a new PR, I will make commits in my repository for you to analyze all of this. Is it okay for you?

Actually it would be easier to start a draft PR. Then we could discuss code / plans / ideas / issues there.

No border base theme here : f8694bd

Thanks, this looks good!
Some minor things:

  • no shadows (as you mentioned)
  • emoji tab switcher background is darker than it should be (seems to be same color as with light theme). I see you set categoryPageIndicatorBackground to #F4F4F5, so I don't know why this happens. But this is a very minor issue, we can ignore it for now
  • the more keys popup has a different shape than for other themes:
    morekeys

Shadows and more keys shape should be the same as for light theme, at least initially.
Once the base themes are implemented, shadows and more keys can be done in new PRs. Those should add settings to switch between different styles (I should be able to help if you have issues with this).

@BlackyHawky
Copy link
Contributor Author

Everything is fixed in this commit: d40828e

  • Shadows were added the same way as the light theme;
  • Emoji tab switcher background is now white;
  • The key popup has a similar shape like other themes.

Can you tell me if this suits you? For my part, I no longer see any error.
If so, I should be able to start a new PR today or tomorrow and start the basic border theme.

@Helium314
Copy link
Owner

Can you tell me if this suits you? For my part, I no longer see any error.

I agree, this is definitely good now.

@BlackyHawky
Copy link
Contributor Author

You can consider this PR permanently closed.
More here.

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.

4 participants