[ADD] Icons management #1978
[ADD] Icons management #1978
Conversation
| /* | ||
| * This returns the icon in char | ||
| */ | ||
| wxBitmap GetIcon(std::string icon_name) { |
There was a problem hiding this comment.
This code is too long to be in the .h file. So could you move it to the .cpp file?
| GOIconManager::GOIconManager(){}; | ||
|
|
||
| GOIconManager::~GOIconManager(){}; |
There was a problem hiding this comment.
Could you remove these lines?
| m_ToolBar->AddTool( | ||
| ID_AUDIO_MEMSET, | ||
| _("&Memory Set\tShift"), | ||
| GetImage_set(), | ||
| m_icon->GetIcon(set), |
There was a problem hiding this comment.
| m_icon->GetIcon(set), | |
| m_icon->GetIcon("set"), |
| @@ -274,16 +276,19 @@ GOFrame::GOFrame( | |||
|
|
|||
| m_ToolBar = CreateToolBar(wxNO_BORDER | wxTB_HORIZONTAL | wxTB_FLAT); | |||
| m_ToolBar->SetToolBitmapSize(wxSize(16, 16)); | |||
| GOIconManager *m_icon = new GOIconManager(); | |||
There was a problem hiding this comment.
| GOIconManager *m_icon = new GOIconManager(); | |
| GOIconManager iconManager; |
| @@ -274,16 +276,19 @@ GOFrame::GOFrame( | |||
|
|
|||
| m_ToolBar = CreateToolBar(wxNO_BORDER | wxTB_HORIZONTAL | wxTB_FLAT); | |||
| m_ToolBar->SetToolBitmapSize(wxSize(16, 16)); | |||
| GOIconManager *m_icon = new GOIconManager(); | |||
| std::string set = "set"; | |||
There was a problem hiding this comment.
Could you remove this line? As well as the other std::string variables?
| /* | ||
| * This returns the icon in char | ||
| */ | ||
| wxBitmap GetIcon(std::string icon_name) { |
There was a problem hiding this comment.
Please make this function static
|
@larspalo Dou you agree with the new icons? |
@oleg68 @rousseldenis Unfortunately, I think that some of the suggested icons might a bit better and some are much worse. I've not yet taken the time to investigate if there are better options within the art library that's used for the ones I really don't like from what @rousseldenis has picked. Also I note that it seems that the wx 3.0 builds are broken with this PR which is un-acceptable in itself. |
Yes, as said, this is a draft proposal to enhance ux in a coherent way of icons representations. Indeed, I need to see what is incompatible with wx 3.0 (I think I've read it was). But, the library makes the executable growing to 120 Mo which is quite unacceptable. So, at the end, I should replace the library import by static chars icons. Nevertheless, @larspalo, this is a proposal, we can adjust the ones we want at the end. We can also pick another style or another library. |
Sure. I'm not at all against a refreshing of the UI. But, in my opinion, something isn't necessarily better just because it's "new". When things are getting closer to a real suggestion/PR I'll look into it further. |
This is a naive implementation of how icons can use the excellent wxMaterialDesignArtProvider.
https://github.com/perazz/wxMaterialDesignArtProvider