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

[Gui] New Icons and rearrangement of Draw Style menu #3119

Merged
merged 2 commits into from Mar 12, 2020

Conversation

carlopav
Copy link
Contributor

@carlopav carlopav commented Mar 1, 2020

Just an experiment, didn't try to compile
Gui: Changed icons in Draw Style menu
And also saved old icons in pure svg.

@carlopav carlopav changed the title [Gui] New Icons and rearrangement of Draw Style menu [Gui] [do not merge] New Icons and rearrangement of Draw Style menu Mar 1, 2020
@triplus
Copy link
Contributor

triplus commented Mar 6, 2020

I see you have changed the command object names. In that case i will have to update the macros:

https://forum.freecadweb.org/viewtopic.php?f=3&t=14336&start=50#p364692

P.S. Not a big deal, just wondering if it is necessary to change the object names in addition to changing the icons.

@carlopav
Copy link
Contributor Author

carlopav commented Mar 6, 2020

I see you have changed the command object names.

Hmmm this was not my intention, I just wanted them to appear in the menu in the order from the most simple (points) to the most complete (flat lines), so I thought to re-order them in every list of the file, but I probably did something wrong...

@triplus
Copy link
Contributor

triplus commented Mar 6, 2020

My bad. You didn't use different object names, you have just moved them around a bit. In that case the mentioned macros should continue to work just fine.

Sorry for the noise.

@carlopav
Copy link
Contributor Author

carlopav commented Mar 6, 2020

Anyway, if you know someone who can test it and correct whatever needed, feel free to add him to the conversation :)

a6->setCheckable(true);
a6->setIcon(BitmapFactory().iconFromTheme("DrawStyleWireFrame"));
a6->setObjectName(QString::fromLatin1("Std_DrawStyleNoShading"));
a1->setCheckable(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely this should be a6 instead of a1.

@triplus
Copy link
Contributor

triplus commented Mar 6, 2020

In general it looks OK to me. I added a note, as one line needs changing. I see you have already discussed the proposed change on the forum.

P.S. After you will update the PR best to change the title to indicate this PR is ready to be merged.

Just an experiment, didn't try to compile
Gui: Changed icons in Draw Style menu

And also saved old icons in pure svg.
@carlopav
Copy link
Contributor Author

carlopav commented Mar 6, 2020

Good catch! i updated and rebased, but the point is that i didn't try to compile it... do you think it's safe to merge it anyway?

@sgrogan
Copy link
Contributor

sgrogan commented Mar 6, 2020

Travis will compile it for you. https://travis-ci.org/FreeCAD/FreeCAD/builds/659359801?utm_source=github_status&utm_medium=notification

@sgrogan sgrogan closed this Mar 6, 2020
@sgrogan sgrogan reopened this Mar 6, 2020
@triplus
Copy link
Contributor

triplus commented Mar 6, 2020

One additional thing, beyond changing the title, you should do is to add newly introduced icon (DrawStyleHiddenLine.svg) to:

https://github.com/FreeCAD/FreeCAD/blob/master/src/Gui/Icons/resource.qrc

The rest looks OK to me.

Reference:
https://forum.freecadweb.org/viewtopic.php?f=34&t=43579

@carlopav carlopav changed the title [Gui] [do not merge] New Icons and rearrangement of Draw Style menu [Gui] New Icons and rearrangement of Draw Style menu Mar 6, 2020
@carlopav
Copy link
Contributor Author

carlopav commented Mar 6, 2020

Done, Thanks @triplus and @sgrogan

@yorikvanhavre yorikvanhavre merged commit 9a36a37 into FreeCAD:master Mar 12, 2020
@carlopav carlopav deleted the GuiReorderDrawStyle branch March 12, 2020 12:33
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.

None yet

4 participants