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 icon for Bambustudio #3910

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

Conversation

wilwarindi
Copy link

As per request #3899

@achadwick
Copy link
Contributor

Can the logo design be made pure #ffffff? I know the docs say that white is #e4e4e4, but in practice it looks rubbish for the designs that are drawn on top of icons, and nobody does it. The e4 bit is probably just about "paper" shapes rather than "ink" shapes.

(Yes, somebody should fix the docs... sigh. I've tripped over this myself.)

@achadwick
Copy link
Contributor

Your designs are great, by the way! Forgive the slightly judgemental tone above, it's just me griping about the state of the documentation, and being misled by it myself 🙇‍♂️

The light e4 grey colour really looks quite dim against a coloured background, and stands out far less than it should. Certainly in the mimetypes folder at least, all or most of the generic fallback icons use pure ff white designs on coloured backgrounds.

@wilwarindi
Copy link
Author

Hey, thank you for the feedback! And no apology needed, I understand this is all done on our free time and for no pay, so documentation tends to lag behind. I'll update the colors then. I also don't like the e4 color, but was following directions. So I'm am happy I can change it now!
I was mostly concerned because of the pixel alignment, and it seems I got it right now.

@morganist
Copy link
Member

i can't merge unfortunately, because there are still elements that use matrix transform

@wilwarindi
Copy link
Author

i can't merge unfortunately, because there are still elements that use matrix transform

How do I fix that?

@achadwick
Copy link
Contributor

@wilwarindi
Have a look at https://stackoverflow.com/questions/13329125/removing-transforms-in-svg-files. In this case, I only needed to ungroup everything.

I noticed a couple of other snags.

  1. There were a few duplicate, blank objects. I've deleted them.
  2. The top highlight of the base shape is a little misaligned, and seems a bit wide in the 64px.
  • Try copying the base shape, pasting in place (Ctrl+Alt+V), changing colour to '#ffffff' and opacity to 20% (10% for dark objects), then copy-pasting that in place, moving the third copy down by 1 pixel (0.5 for the 22px and 24px, and you don't do this for the 16px), then selecting bottom white, top white, and choosing Path → Difference from the menu. Obviously, they all have to be paths to start with.
  1. There's no shadow under the base shape.
  • I create these by paste-in-place, setting them to pure black, opacity 20% (always), offsetting them down by 1 or 0.5 pixels depending on the icon size, and using PgDn to move them down to the right place in the stack.

I've fixed them up on a branch, and I'll send you a branch reference you can pull from onto this PR (or, that somebody with commit rights can, anyway).

achadwick added a commit to achadwick/papirus-icon-theme that referenced this pull request Jan 18, 2025
- neatened the highlight by path difference
- added a shadow under the main shape
- removed some duplicate objects
- snapped some more edges to exact pixel boundaries

Suggestions for PapirusDevelopmentTeam#3910
@achadwick
Copy link
Contributor

@wilwarindi
Copy link
Author

@achadwick awesome, thanks for the help! I did notice the blank objects - they were supposed to be shadows, but I guess the "prepare" script nixed them. Oh, well. This has been a steep, but fun learning curve.
Now, I don't know how to add your suggestions to the PR other than downloading it and re-uploading it from my pc. Is that the only way? I always assume there's a simpler option I just don't know about. I'll try that now.

@achadwick
Copy link
Contributor

@wilwarindi
I think there's a way of pulling a little change like that into a PR branch like this one just with the URL. I'm a bit rusty on how though, since it's been a while since I did anything like that regularly. git really does have a steep learning curve, on both sides of the mountain 😅

Anyway, thanks for integrating what I did. It doesn't matter how the changes get merged, but everything is neat and free of matrix transforms. I think this PR branch is ready now as of 37624e3. @morganist, are you able to merge it? Full credit to @wilwarindi for the designs.

@achadwick
Copy link
Contributor

Quick aside: I've documented "logos and stuff on a coloured background" as an exception to the general rule for the #e4 "white": see #3912

Really the e4 grey-white is a guide for "object" colours - sheets of paper, etc. Perhaps that could even be clarified further (comment on the #3912 if you want to make a style guide suggestion :))

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