#421 Feature/Patch: Add dialog for global variables to "Edit Path" dialog

[bluehazzard]

Hi,
i added a dialog to easily select global variables as paths

there is a small quirck and i can't figure out how to solve it, any help is apreciated:
the minimal size of the dialog seems to be to small, and i don't know hot to fix this without adding a hard coded minimal size..

[bluehazzard]

here a image:

[bluehazzard]

updated patch. There were wx3.0 issues

[Teodor Petrov]

Minimal size is the only way to go as far as I know.
Good idea btw.
Please make the button more readble by not using abbreviations.

[bluehazzard]

Please make the button more readble by not using abbreviations.

i agree with this, but to what? i can't write "Global user variables" or simply "Var"? or double line button? i don't like 5cm long buttons ;)

[Teodor Petrov]

One option is to use some bitmap button and fine some suitable icon.

[bluehazzard]

How about this:

[Teodor Petrov]

It is two step opeation as far as I can see. But I think you can use a wrench type icon for the button.

[bluehazzard]

Yes it is a two way operation. The intended look was like the drop down menus in firefox toolbar, or other applications. But wxWidgets does not have a combined button/arrow down menu and i am not in the mood to write one by hand. With this approach with a additional menu there is also place for other options in the future..
I have also fixed the issue with the minimal size of the dialog...
Next thing i do is some logic to mix the browse button with the global variables for right sub folder browsing. Then i will post a patch again

[bluehazzard]

Next thing i do is some logic to mix the browse button with the global variables for right sub folder browsing

this was already implemented :)
So here is my proposal for the final patch

patch is always made against trunk

[Teodor Petrov]

Will you remove the menu? Or you'll leave it to me to do it?

1. No abbreviations everywhere please things like OnUsrVar or GetUsrVarDialog are unacceptable.
2. You're missing spaces after commas.
3. There are plenty of places where you've used () instead of wxT(). () means translate this string and this will break the code if someone does translate it.
4. Use camelCase for variable names, not lower_case_and_underscores.
5. Some places use wxString a instead of const wxString &a.
6. I saw additional ; at the end of inlince class method. This semicolon is useless.
7. Missing spaces after if and for.
8. UserVariableManager::GetVariable - what happens if the user closes the dialog without choosing a variable?

[bluehazzard]

No abbreviations everywhere please things like OnUsrVar or GetUsrVarDialog are unacceptable.

Fixed it for API calls. Local variables are still shot (like dlg, i think this improves the readability)

You're missing spaces after commas.

Fixed

There are plenty of places where you've used () instead of wxT(). () means translate this string and this will break the code if someone does translate it.

thought it was the other way around.... Fixed

Use camelCase for variable names, not lower_case_and_underscores.

Done

Some places use wxString a instead of const wxString &a.

i think i fixed this too, not 100% sure if I found all places

I saw additional ; at the end of inlince class method. This semicolon is useless.

fixed

Missing spaces after if and for.

fixed

UserVariableManager::GetVariable - what happens if the user closes the dialog without choosing a variable?

will return an empty string -> nothing selected

Will you remove the menu? Or you'll leave it to me to do it?

i think the menu is the most intuitive way to handle this. The arrow is the universal symbol for other functionality and the menu entry explains exactly what is going on. A wrench symbols option and not user variables ;) . If you don't like it, remove it...

[Morten MacFly]

Good one. I like it pretty much.
@Teodor: Feel free to apply.

[Morten MacFly]

After looking into the code I've two notes:
1.) I would have preferred not to use XRC but wxSmith instead.
2.) You shoudl never setup a specific size in UI (XRC) files. This will definetely spoil the layout on certain platforms. The only thing that may make sense it to use the minsize attribute. As a rule of thumb: Use <size> only if you intend to use it on a single platform only (which certainly not the case for C::B).

So please just remove the <size> tags (2 occurernces in total) from the XRC file. If you have too much time, make a wxSmith out of it. :-)

Besides that it looks fine... currently testing...

[Teodor Petrov]

Morten MacFly: If you have time to remove the menu and change the icon, go ahead and apply it. I don't have it at the moment.

[Morten MacFly]

Done.
Applied in SVN (modified). Thanky you!

[White-Tiger]

now, if one could implement the suggested drop-down menu items to enhance/improve usability or otherwise fix the dialog, it would be awesome.
The current implementation is quite unusable on Windows.. it's like 19 characters for the path input (not enough to get a good picture) and it's not resizable... or rather the input box won't resize.. Also, both buttons are now almost the same size as the input field.. doesn't look good either.

[Teodor Petrov]

This problem is not only on windows. :(

[Teodor Petrov]

Fix the layout issue. Switched to bitmap buttons. Fixed a out-of-bounds bug. Added sorting. Switched to using wxStdButtonSizer.

Report if you find any problem with the new version, because I've not tested it on windows.
I'd be grateful if someone posts a screenshot from it.

[White-Tiger]

This is how it looks on Windows. (default size)
And I can confirm that resizing works now as it should. Though I'm not really happy with the button icons... especially the last one looks odd to me.

[Teodor Petrov]

The problem is that there is no good way to choose a pleasing icon. :(