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

Next_Nightly
fixed
None
Patch
2017-01-15
2016-10-27
bluehazzard
No

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..

1 Attachments

Discussion

  • Teodor Petrov

    Teodor Petrov - 2016-10-27

    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

    bluehazzard - 2016-10-27

    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

    Teodor Petrov - 2016-10-27

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

     
  • Teodor Petrov

    Teodor Petrov - 2016-10-30

    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

    bluehazzard - 2016-10-30

    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

    bluehazzard - 2016-10-30

    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

    Teodor Petrov - 2016-10-31

    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

    bluehazzard - 2016-11-03

    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...

     
    Last edit: bluehazzard 2016-11-03
  • Teodor Petrov

    Teodor Petrov - 2016-11-04
    • assigned_to: Teodor Petrov
     
  • Morten MacFly

    Morten MacFly - 2016-11-06

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

     
    Last edit: Morten MacFly 2016-11-06
  • Morten MacFly

    Morten MacFly - 2016-11-06

    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

    Teodor Petrov - 2016-11-06

    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

    Morten MacFly - 2016-11-06
    • status: open --> applied
    • assigned_to: Teodor Petrov --> Morten MacFly
    • Milestone: Undefined --> Next_Nightly
     
  • Morten MacFly

    Morten MacFly - 2016-11-06

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

     
  • White-Tiger

    White-Tiger - 2016-11-15

    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

      Teodor Petrov - 2016-11-16

      This problem is not only on windows. :(

       
  • Teodor Petrov

    Teodor Petrov - 2016-12-04
    • status: applied --> open
     
  • Teodor Petrov

    Teodor Petrov - 2017-01-14
    • status: open --> fixed
    • assigned_to: Morten MacFly --> Teodor Petrov
     
  • Teodor Petrov

    Teodor Petrov - 2017-01-14

    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

    White-Tiger - 2017-01-15

    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

      Teodor Petrov - 2017-01-15

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

       

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks