-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Update Draft WB icons #13585
Update Draft WB icons #13585
Conversation
Some background info:
With that in mind I would not change Draft_Annotation_Style.svg and Draft_Apply.svg. The added sheet does not have the 'manager' connotation for me. But I am not a GUI specialist. Regarding the blue pencil icons: |
Hi @Roy-043 , thank you for the feedback and some background !
Ok, so I removed Draft_Annotation_Style and Draft_Apply changes from this PR. We can always revisited them later when the changes you propose above are implemented.
I made the TechDraw one consistent too.
I improved them regarding this aspect and cleaned the pencil paths, also making it symmetrical.
I tweaked all icons using the pencil I could find, like Std_UserEditModeColor, Std_UserEditModeCutting.svg, Std_UserEditModeDefault.svg, Std_UserEditModeTransform.svg. Also simplified the scissors and palette paths while I was on it. I'd like to squash all my commits into one, so it's easier to compare only once all changes, but I do not succeed to do it on my end with GitHub Desktop (I get this error "Unable to squash. Squashing replays all commits up to the last one required for the squash. A merge commit cannot exist among those commits"). |
You have changed more icons, I did not expect that, but doing so does make sense. Thanks. I have two remaining comments related to the Draft icons: Other than that this looks just fine to me. Thanks again. |
Hi @Roy-043 thank you for the comments !
I now reused a similar-ish shape to the original one.
Wow, you have an accurate vision, lucky you ! =D I tweaked them so it's better distributed, in all 3 icons showing a pile of layers.
I don't know what's best about these changes touching separate "areas" ? Should I make 3 different PR ? |
This PR does not follow the prescribed procedure. I do not think that is a big issue here (everybody still has to get used to the new rules anyway). IMO this PR can be merged. I am not going to do that myself since this PR is not restricted to the Draft WB. The final call should be made in the next merge meeting (usually on Monday). Thanks again. |
@marcuspollio Draft_...
could be revamped too, they are too thin and have no highlight/shadow to be visible in light and dark themes (compare e.g. with Draft_Offset) |
Hi @maxwxyz
I tweaked them too in the last commit. |
Nice. Especially the ellipse icon is currently bad. But maybe the Bezier curve icon could be improved as well - 3 is barely visible there. Like 1.00 in Dimension. |
Hi @FEA-eng , thanks =D
What would you suggest ? On my side, it's fairly ok : |
…rces Update texts for visibility Some more cleanup Use consistent FreeSans font
eb7322b
to
40ed0ff
Compare
In any case I guess it's better than what we have now, so I wold merge this already, and we work further :) Unless you want to do some more here, @marcuspollio ? |
Slight adjustments to some Draft WB icons for better consistency :
Also fix some Core and TechDraw WB icons that use the same elements for consistency.
Also save icons as Plain SVG, cleanup unused resources and add a bit of metadata when missing.