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

Code review request #29

Closed
GoogleCodeExporter opened this issue Mar 22, 2015 · 14 comments
Closed

Code review request #29

GoogleCodeExporter opened this issue Mar 22, 2015 · 14 comments

Comments

@GoogleCodeExporter
Copy link

I'm going to merge my code into trunk. Please review rev. 239 to be sure
that nothing is broken.

Implemented features:
 Map/Satellite/Terrain switch
 Batch download from the map GUI

I know that the download dialog looks somewhat ugly, and it would be great
if somebody fixed it.

Original issue reported on code.google.com by Maxim.Ra...@gmail.com on 31 Mar 2009 at 11:16

@GoogleCodeExporter
Copy link
Author

First test on Windows 
  After updating the files the Main "form" will not show up :(, I have not look into
the code yet, will try to find out in a few...

Original comment by heldersepu on 31 Mar 2009 at 12:40

@GoogleCodeExporter
Copy link
Author

It seem that windows does not like this line:
 gtk.gdk.threads_init()

some more info about this:
 http://www.daa.com.au/pipermail/pygtk/2007-July/014049.html
 http://www.nabble.com/Application-crashes-when-appending-text-to-gtk.ComboBox-while-it-got-the-focus-(MS-Windows)-td198791.html
 http://faq.pygtk.org/index.py?req=show&file=faq14.023.htp

Original comment by heldersepu on 31 Mar 2009 at 1:28

@GoogleCodeExporter
Copy link
Author

Yes, I've already found it, and now looking for a workaround.

Original comment by Maxim.Ra...@gmail.com on 31 Mar 2009 at 1:40

@GoogleCodeExporter
Copy link
Author

To be completely honest I do not like the way it looks with the download button
(DwnBtn.PNG) 
What about a less invasive approach, like a new menu item (DwnMnu.PNG) in that 
way we
could even send the coordinates to the "DLWindow"

 Also it will be a best to take the "class DLWindow(gtk.Window)" out of maps.py into
it's own file, something similar to mapTools.py

Original comment by heldersepu on 31 Mar 2009 at 2:05

Attachments:

@GoogleCodeExporter
Copy link
Author

Added a couple of crutches for win32, now it seems to work - see rev.249

Original comment by Maxim.Ra...@gmail.com on 31 Mar 2009 at 2:18

@GoogleCodeExporter
Copy link
Author

I agree that the class DLWindow should be moved into a separate file, but it 
doesn't
look urgent. Moreover, the downloader thread itself should be extracted into a
separate class and reused in the main window and in download.py 

As for a popup menu, it's not a good idea because it hides functionality from 
users.
Actually, until today I didn't even know that popup menu exists in the current 
version.

Original comment by Maxim.Ra...@gmail.com on 31 Mar 2009 at 2:50

@GoogleCodeExporter
Copy link
Author

I totally agree with you! 

 - The "downloader thread" should indeed be a separate class
 - The PopUp menu might be good as a anther way to open the "DLWindow", but we
definitely want to make the download functionality very visible!

I'm looking to see if the download button looks better a line below, with the 
check
boxes, it seems that the combo box that holds the layers is a lot bigger than 
what it
needs...

Original comment by heldersepu on 31 Mar 2009 at 4:07

Attachments:

@GoogleCodeExporter
Copy link
Author

This could be another option...

Original comment by heldersepu on 31 Mar 2009 at 4:38

Attachments:

@GoogleCodeExporter
Copy link
Author

I'm not a great guru in GUI, all positions of a download button look equivalent 
for me.

Original comment by Maxim.Ra...@gmail.com on 31 Mar 2009 at 8:31

@GoogleCodeExporter
Copy link
Author

If all the position are equivalent for you, then I will put it in the same row 
of the
check boxes, in that way the "Entry" box is wider. 

Original comment by heldersepu on 31 Mar 2009 at 9:30

@GoogleCodeExporter
Copy link
Author

Just do it ;)

Original comment by Maxim.Ra...@gmail.com on 31 Mar 2009 at 10:36

@GoogleCodeExporter
Copy link
Author

I been testing it for a while and it all looks ready to merge! 

Original comment by heldersepu on 1 Apr 2009 at 1:42

  • Changed state: Started

@GoogleCodeExporter
Copy link
Author

Merge complete

Original comment by Maxim.Ra...@gmail.com on 1 Apr 2009 at 8:45

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

Original comment by heldersepu on 12 Aug 2009 at 1:53

  • Changed state: Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant