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

Add new vector icon #592

Merged
merged 3 commits into from
Mar 31, 2024
Merged

Add new vector icon #592

merged 3 commits into from
Mar 31, 2024

Conversation

the-eclectic-dyslexic
Copy link
Contributor

fixes #517

user themes off
notheme

user themes on
theme

Tested in android studio using an emulator for (Android 14) and fairphone 4 (Android 13)

@Helium314
Copy link
Owner

Thanks!
What are the asstes/launcher_icon files used for? As far as I know, Andorid doesn't support svg files directly.
The playstore icon seems unnecessary too, but it doesn't really matter as it should not be added to the apk anyway.

@Helium314
Copy link
Owner

@the-eclectic-dyslexic
Copy link
Contributor Author

the-eclectic-dyslexic commented Mar 29, 2024

@Helium314 The assets/launcher_icon is the source svg in case you want to reimport/modify/whatever. Would you like them somewhere else?

Sure I can update that. I am just talking with the designer a little, making sure they are happy to ship the changes we have been talking about, to improve readability, and once that is wrapped up I will make sure to fix all those problems along with updating the visuals. I can remove the playstore icon if you like, and I will make sure to update the fastlane icon.

@the-eclectic-dyslexic
Copy link
Contributor Author

I hope that is an improvement!

Let me know if I missed anything, or there are any additional concerns.

@the-eclectic-dyslexic
Copy link
Contributor Author

the-eclectic-dyslexic commented Mar 30, 2024

And Just in case you haven't been following #517 where this was discussed with the artist, here are some screen shots of the current build, tested the same way as the first commit.
theme
no_theme

@Helium314
Copy link
Owner

Thanks for the update!
I think this is definitiely an improvement over the previous style.

@BlackyHawky
Copy link
Contributor

This is just a detail, but don't forget to update the Credits section of the Readme with your name and the artist's name. 😉
Can't wait to test on my devices!

Perhaps the license headers of new files can be updated by adding :
<!-- SPDX-License-Identifier: GPL-3.0-only -->

What do you think Helium?

@Helium314
Copy link
Owner

Actually the license should be choice of the artist (but compatible, obviously).
@FabianOvrWrt maybe some Creative Commons license?

@FabianOvrWrt
Copy link

Actually the license should be choice of the artist (but compatible, obviously). @FabianOvrWrt maybe some Creative Commons license?

Yeah, I was thinking about http://creativecommons.org/licenses/by-sa/4.0/ which basically means "as long as you credit me, you can use or modify the work as you please".

@FabianOvrWrt
Copy link

Also, checking the PR I still see the font is too thin (not 750 in weight as I proposed). Here's an updated svg with the proper proportions I was going for in case @the-eclectic-dyslexic couldn't figure out my mess last time:

HeliBoard_4

@the-eclectic-dyslexic
Copy link
Contributor Author

the-eclectic-dyslexic commented Mar 31, 2024

Okay, I should have that done in an few hours. Then hopefully heliboard can have a shiny new hood ornament. 🙂

The only things to keep in mind are the export svgs won't automatically have the license referenced in the comments... and probably not the meta data if we go that route. If anyone saves the source svg in inkscape as a "minimized svg" the license will disappear from anywhere I put it, unless I literally make it an element floating off canvas.
edit: I was wrong, inkscape is smarter than I remember. Both are preserved on export to plain svg, and upon saving as "Optimized SVG"

I will put it in a comment, and meta data, of the source svg and exports for now. I will also put a copy of the creative commons license with the other licenses, and update the readme as suggested.

Sorry about that @FabianOvrWrt I did check the font weight, I think I just misread the UI? I'll try to figure out what happened there.

@the-eclectic-dyslexic
Copy link
Contributor Author

the-eclectic-dyslexic commented Mar 31, 2024

I figured out the font issue. I had the static fonts installed, not the dynamic weight ones. I used the font weight annotation included, but as a consequence of having the wrong version of the font installed it did precisely nothing. The silly thing is I had a passing suspicion about that, but just figured "nah!". My bad! 😵

I have used the version where the text is forced to paths this time, to avoid any poor unsuspecting soul falling into the same trap in the future.

I updated the readme to say the icon is by Fabian "with contributions from" myself. I hope you find that wording accurate @FabianOvrWrt

I also added a link in the readme to an included CC-BY-SA-4.0 plaintext license and why it is there. The creative commons suggests linking directly to https://creativecommons.org/licenses/by-sa/4.0/ so I included that link in the readme as well.

The SVGs should carry forward their licenses, both in the SPDX identifier and the meta data, unless someone explicitly removes them. It should even carry forward automatically when someone exports SVGs for the layers to import into android studio. (edit: if the export is done in inkscape 1.3.2 or later. I can't speak to other vector editors)

@the-eclectic-dyslexic
Copy link
Contributor Author

notheme
theme

@the-eclectic-dyslexic
Copy link
Contributor Author

The only other thing I can suggest here is I could squash the commits to prevent any files from not having licenses at any point in the project history, and it would also get rid of some binaries.

If the record is more important though, or no one minds either way, then I won't bother.

@Helium314
Copy link
Owner

Thanks a lot @the-eclectic-dyslexic and @FabianOvrWrt
I usually squash PR commits anyway, unless there is a good reason not to.

@Helium314 Helium314 merged commit 252b435 into Helium314:main Mar 31, 2024
RHJihan pushed a commit to OishikStudio/Probhat-Keyboard that referenced this pull request Apr 1, 2024
RHJihan added a commit to RHJihan/HeliBoard that referenced this pull request Apr 2, 2024
RHJihan added a commit to OishikStudio/Oishik-Keyboard that referenced this pull request Apr 4, 2024
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.

New Icon + Dynamic Icon Colour
4 participants