brainlid commented 13 years ago

You can close this if you like, just wanted to share my observations on some design limitations and missing features to an otherwise very nice library. I was hoping to use your library and when I tried it out I found the following issues:

Each marker creates a MarkerImage. There is no reuse of MarkerImages. I may have 6 image types shared between 30 markers but they are all duplicated. (See Cartographer's ability to declare Markers)
While Markers can be made draggable, I saw no way to attach an event to the drag-end to hook into and update the server. May have just missed it.
No shadow images possible for markers.
MarkerImage anchors don't seem to support pixel locations. I use these custom map icons (http://mapicons.nicolasmollet.com/) and need the ability to specify pixel specific locations.

There are a number of things I like about your library that are better than Cartographer. I didn't want to try and add any of those features as it would require significant API changes. What are your thoughts/plans for the library and these features?

apneadiving commented 13 years ago

Hi Mark,

I won't close your valuable remarks, they really worth reading.

Let's review point after point:

Duplicate markers: You're definitely right, I even didn't think about that... It should be easy to fix
Draggable markers + events: I can't see any generic way to provide this functionality. You're very welcome to suggest it. From now, I created gmaps4rails_callback for this kind of purpose
Shadows: You're right, I'll add it soon
Anchors: the anchor param available in markers options has been created by a contributor. Maybe you could extend it's behaviour.

To sum up:

I'll do 1 & 3
I need brainstorming about 2
I let you do 4

Thoughts?

Benjamin

apneadiving commented 13 years ago

just did 1 & 3.

It's late but I think I'm gonna continue with 2 during the week.

brainlid commented 13 years ago

Very cool! I'll check out what you did and see what I can do about #4 (anchors). I will also look into your callback mechanism to see if that works for my needs with #2 (drag-end event).

I'm impressed with your response. Thanks! -Mark Ericksen

apneadiving commented 13 years ago

Just realized a typo in my previous post. I'll do 4 this week.

Just consider thinking about 2 then :)

apneadiving commented 13 years ago

Just added anchors. Could you please test?

 def gmaps4rails_marker_picture
 {
  "picture" => "/images/#{name}.png",
  "width" => "20",
  "height" => "20",
  "marker_anchor" => [ 5, 10],
  "shadow_picture" => "/images/morgan.png" ,
  "shadow_width" => "110",
  "shadow_height" => "110",
  "shadow_anchor" => [ 5, 10],
 }
end

brainlid commented 13 years ago

Got some time to play with your latest changes tonight.

Having some problems but I'm not sure what the issue is yet.

Installation problem?

Setup:

Rubygems 1.3.7
Gemfile line \ gem 'gmaps4rails', :git => 'git://github.com/apneadiving/Google-Maps-for-Rails.git'

Error Message:

Using gmaps4rails (0.8.8) from git://github.com/apneadiving/Google-Maps-for-Rails.git (at master) gmaps4rails at /home/mark/.rvm/gems/ruby-1.8.7-p302@mcp/bundler/gems/Google-Maps-for-Rails-2c7cb38303c1 did not have a valid gemspec. This prevents bundler from installing bins or native extensions, but that may not affect its functionality. The validation message from Rubygems was: ["lib/acts_as_gmappable/base.rb", "lib/array.rb", "lib/gmaps4rails_helper.rb", "lib/hash.rb", "test/dummy/spec/controllers/users_controller_spec.rb", "test/dummy/spec/factories.rb"] are not files

When I run the code that builds an array of markers, the following error occurs: "undefined method `to_gmaps4rails' for #Array:0x7fe203354818"

That code looks like this:

map_objects = []
map_objects += UserLocation.accessible_by(current_ability).geo_scope(:within => 5, :origin => current_user.position).where("user_id <> #{current_user.id}").includes(:user).all
map_objects += Customer.accessible_by(current_ability).geo_scope(:within => 3, :origin => current_user.position).all
@map_json = map_objects.to_gmaps4rails

I have confirmed that it is correctly Array of valid objects.

The same code ran fine using the 'official' 0.8.8 (not github master). So, I'm not sure if I did something wrong or if something else changed. I see where you are opening Array and extending it, but somehow it isn't getting picked up. I've restarted the app server a couple times to make sure.

Sorry to slow down your very rapid development progress. :D

apneadiving commented 13 years ago

:) Seems it's my fault...

Weird though: it works in local using :path. could you try?

Otherwise, I'll rebuild the gemset tonight.

Cheers,

Ben

brainlid commented 13 years ago

Sorry, I don't know what you mean for me to try. On Jun 28, 2011 2:20 AM, "apneadiving" < reply@reply.github.com> wrote:

:) Seems it's my fault...

Weird though: it works in local using :path. could you try?

Otherwise, I'll rebuild the gemset tonight.

Cheers,

Ben

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1453067

apneadiving commented 13 years ago

Sorry:

To save one day because I'm not able to patch now, you could try again this way:

clone the repository on your disk
in your gemfile, include the gem this way:

gem 'gmaps4rails', :path => "path from your app to the cloned repository"

Hope it's clearer.

brainlid commented 13 years ago

That makes sense. Thanks. Will try in a couple hours. On Jun 28, 2011 7:23 AM, "apneadiving" < reply@reply.github.com> wrote:

Sorry:

To save one day because I'm not able to patch now, you could try again this way:

clone the repository on your disk

in your gemfile, include the gem this way:

gem 'gmaps4rails', :path => "path from your app to the cloned repository"

Hope it's clearer.

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1455422

brainlid commented 13 years ago

Hmm. Just tried it. Same issue, it isn't picking up the Array extension. I cloned the repo and included in the gemfile via :path.

I even tried making some 'require' changes in lib/gmaps4rails.rb but no change.

I'm on Rails 3.0.5

-Mark

On Tue, Jun 28, 2011 at 7:26 AM, Mark E. mnmfactory@gmail.com wrote:

That makes sense. Thanks. Will try in a couple hours. On Jun 28, 2011 7:23 AM, "apneadiving" < reply@reply.github.com> wrote:

Sorry:

To save one day because I'm not able to patch now, you could try again this way:

clone the repository on your disk

in your gemfile, include the gem this way:

gem 'gmaps4rails', :path => "path from your app to the cloned repository"

Hope it's clearer.

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1455422

apneadiving commented 13 years ago

Ah, never mind...

I've moved and reorganized many files and haven't rebuilt the gemspec yet. Strange it works on my mac though.

I'll tell you when this is fixed.

apneadiving commented 13 years ago

gemspec regenerated, should be good now (hopefully)!

brainlid commented 13 years ago

Still having problems. Sorry!

I switched back to the 0.8.8 published gem to confirm that it would work. It worked as expected (no shadows obviously).

I pulled the latest code from master and included it using :path. The same problem happens.

I removed the local code and tried pulling using :git from master.

bundle install: ... Using gmaps4rails (0.8.8) from git:// github.com/apneadiving/Google-Maps-for-Rails.git (at master)

Same issue.

Sorry, I wish I could figure out more.

-Mark

On Tue, Jun 28, 2011 at 11:00 AM, apneadiving < reply@reply.github.com>wrote:

gemspec regenerated, should be good now (hopefully)!

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1457139

apneadiving commented 13 years ago

Damn :)

Hard to debug since I don't have the problem. I'll investigate before releasing then.

Thanks anyway.

Ps: any idea about point 2?

apneadiving commented 13 years ago

I'm wondering if it could be a conflict with your system's gem because both have version 0.8.8...

Indeed, everything works fine for me in all cases: including gem from git or with :path

brainlid commented 13 years ago

I thought of that too. I'm using RVM, I uninstalled the 0.8.8 version and verified it didn't show in my gem list. I did each time I tried either a :path or :git source. It is certainly possible that it is still something on my system. I'm running on Ubuntu 11.04.

Not sure.

-Mark

On Wed, Jun 29, 2011 at 5:09 AM, apneadiving < reply@reply.github.com>wrote:

I'm wondering if it could be a conflict with your system's gem because both have version 0.8.8...

Indeed, everything works fine for me in all cases: including gem from git or with :path

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1466201

brainlid commented 13 years ago

Ben,

Sorry I haven't been able to assist much. I'm currently very busy. Ugh.

At any rate, I wanted to respond to the question of #2:

Draggable markers + events

You had mentioned being open to discuss/brainstorm solutions. I just wanted to pass along a possible solution.

I had created a fork of the cartographer project some time ago to add this feature. That pull request, along with others, languishes being ignored. :( So you can imagine my shock when you responded so quickly. :)

My project fork: https://github.com/markeric/cartographer

You can see my solution with usage examples in the README. It is after the text "Here is an example of handling a marker being dropped.".

The solution basically amounts to this:

Specify a javascript callback function name in the controller that gets put into the view.
The view defines the javascript function that gets called

This works great but I wish there was a better solution. I don't like it because it is "disconnected". The controller code is naming the view function to call. It would be better to set that in the view if possible just to help "keep it together".

Hope that helps in the thinking process.

-Mark

apneadiving commented 13 years ago

Ok, I'll look at it carefuly.

Just released 0.9.0, I haven't found the bug you reported... hope everything is still fine :/

Cheers.

brainlid commented 13 years ago

I'll try it out later today and report back.

-Mark

On Sat, Jul 2, 2011 at 11:42 AM, apneadiving < reply@reply.github.com>wrote:

Ok, I'll look at it carefuly.

Just released 0.9.0, I haven't found the bug you reported... hope everything is still fine :/

Cheers.

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1490009

Mark Ericksen

apneadiving commented 13 years ago

Ok.

I'd do almost what you did:

add listeners callings predetermined methods such as Gmaps4Rails.dragend
users should implement the code in the view

Still, this doesn't seem to bring much: as I said, it's the same as using Gmaps4Rails.callback and loop markers to set the behavior.

brainlid commented 13 years ago

I haven't had a chance to look at your callback yet. If it gives the ability to track which event is firing like a dragend versus dragstart, etc. and a reference to the object being moved (that has some user assigned data with it, then it should be just fine.

If that is already possible, then it might only need an example bit of code for doing it.

-Mark

On Sat, Jul 2, 2011 at 2:58 PM, apneadiving < reply@reply.github.com>wrote:

Ok.

I'd do almost what you did:

add listeners callings predetermined methods such as Gmaps4Rails.dragend

users should implement the code in the view

Still, this doesn't seem to bring much: as I said, it's the same as using Gmaps4Rails.callback and loop markers to set the behavior.

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1490501

brainlid commented 13 years ago

Ben,

I just tested 0.9 and it doesn't work for me. Hoping it was just a corrupted set of gems, I created a new rvm gemset, did bundle install and ran the server. The error is the same:

NoMethodError (undefined method `to_gmaps4rails' for

Array:0x7f5e184bfaa0):

app/controllers/home_controller.rb:10:in `index'

My setup:

ruby 1.8.7 p302
Rails 3.0.5
rubygems 1.3.7

Could the problem be related to the ruby version number or a change with a later rails release?

-Mark

On Sat, Jul 2, 2011 at 3:03 PM, Mark E. mnmfactory@gmail.com wrote:

I haven't had a chance to look at your callback yet. If it gives the ability to track which event is firing like a dragend versus dragstart, etc. and a reference to the object being moved (that has some user assigned data with it, then it should be just fine.

If that is already possible, then it might only need an example bit of code for doing it.

-Mark

On Sat, Jul 2, 2011 at 2:58 PM, apneadiving < reply@reply.github.com>wrote:

Ok.

I'd do almost what you did:

add listeners callings predetermined methods such as Gmaps4Rails.dragend

users should implement the code in the view

Still, this doesn't seem to bring much: as I said, it's the same as using Gmaps4Rails.callback and loop markers to set the behavior.

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1490501

apneadiving commented 13 years ago

Ouch...

What if you use the bundled rails app in /tests/dummy/ ?

brainlid commented 13 years ago

How do I run it? It doesn't have a script folder, and I don't usually run specs from a command line so I'm just trying it.

I tried running a few specs, here's what I did and what I got...

rspec spec/models/user_spec.rb

/home/mark/.rvm/gems/ruby-1.8.7-p302@mcp/gems/railties-3.0.5/lib/rails/application/configuration.rb:88:in read': No such file or directory - /home/mark/.rvm/gems/ruby-1.8.7-p302@mcp/gems/gmaps4rails-0.9.0/test/dummy/config/database.yml (Errno::ENOENT) from /home/mark/.rvm/gems/ruby-1.8.7-p302@mcp/gems/railties-3.0.5/lib/rails/application/configuration.rb:88:in database_configuration' from /home/mark/.rvm/gems/ruby-1.8.7-p302@mcp /gems/activerecord-3.0.5/lib/active_record/railtie.rb:58 from /home/mark/.rvm/gems/ruby-1.8.7-p302@mcp/gems/activesupport-3.0.5/lib/active_support/lazy_load_hooks.rb:36:in `instance_eval' [...]

Same thing for other specs.

I certainly could be doing something wrong, but I don't know what. :/

-Mark

apneadiving commented 13 years ago

It's a standard rails app.

You should bundle and rake db:migrate first.

Then run "rails s" to launch the server

brainlid commented 13 years ago

Weird, I was just the gem from the location where rvm installed the gem and it wasn't the complete source. :/

The test/dummy folder was only partially there. So, I cloned the project locally, created a clean gemset, did bundle install, db:migrate, and "rails s".

It runs fine. I noticed you are using rails 3.0.7. I can try upgrading to that version to see if it fixes it for me. I'll also try upgrading my RVM.

Thanks for your patience. :)

-Mark

On Sat, Jul 2, 2011 at 4:22 PM, apneadiving < reply@reply.github.com>wrote:

It's a standard rails app.

You should bundle and rake db:migrate first.

Then run "rails s" to launch the server

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1490754

apneadiving commented 13 years ago

Thanks for yours :)

Ok please try whatever you want to let me understand why the gem seems to bug sometimes!

brainlid commented 13 years ago

After a few hours of pain and suffering, I'll have to put this off for now. :(

I upgraded to Rails 3.0.9 but had no affect
I upgraded RVM
I installed Ruby 1.9.2 (in RVM) but ran into problem after problem with native gems not building for my Ubuntu 11.04. Giving up on that route for now.
Tried using a newly installed/built 1.8.7 but that fails too other Ubuntu specific RVM issues I can't resolve right now. My current working 1.8.7 was brought over from an older ubuntu version in my home folder (rvm). It works for everything I need. That could somehow be related to the issue for all I know.

Sorry, I just need to work on my project for now.

I want to resolve these issues at some point but I'll have to come back later.

-Mark

On Sat, Jul 2, 2011 at 4:49 PM, apneadiving < reply@reply.github.com>wrote:

Thanks for yours :)

Ok please try whatever you want to let me understand why the gem seems to bug sometimes!

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1490814

apneadiving commented 13 years ago

Ok, thanks for your time anyway :)

It's just logic your projects have the priority!

Just to sum up investigations now:

0.9.0 doesn't work in your app
0.8.8 works in your app
0.9.0 works in the dummy app included in the gem on your computer

Correct?

By curiosity, could you post your gemfile?

apneadiving commented 13 years ago

I think I've understood the problem. 0.9.1 should work fine ;)

brainlid commented 13 years ago

Yes! 0.9.1 works great now. I see you changed the paths for the files in the gemspec.

Thanks for helping to fix it. I hope to be able to try out the shadows and anchors later. Do you have a suggestion on how to use your callback function for handling the dragend event?

Thanks, -Mark

apneadiving commented 13 years ago

Yep files need to be namespaced: the bug in your app was due to another gem using the same name and path lib/extensions/array.rb

Concerning the dragend, I just wrote this: https://github.com/apneadiving/Google-Maps-for-Rails/wiki/Marker-dragging

apneadiving commented 13 years ago

I guess I could close this ticket. Everything ok?

brainlid commented 13 years ago

Sure. Thanks again for your excellent response and help. On Jul 15, 2011 6:18 AM, "apneadiving" < reply@reply.github.com> wrote:

I guess I could close this ticket. Everything ok?

Reply to this email directly or view it on GitHub:

https://github.com/apneadiving/Google-Maps-for-Rails/issues/47#issuecomment-1579315

apneadiving / Google-Maps-for-Rails

Design limitations and missing features #47

Error Message:

Array:0x7f5e184bfaa0):