search

mapzen / ui

1 stars 4 forks source link

first pass at component toggle #38

Closed meetar closed 8 years ago

meetar commented 8 years ago

Resolves https://github.com/mapzen/ui/issues/29

meetar commented 8 years ago

Mostly works – the geolocator's position isn't quite right when the citysearch is hidden, so further css work is necessary. What do you think of the general approach?

meetar commented 8 years ago

@louh sanity check requested - thanks!

louh commented 8 years ago

Hi @meetar ! Apologies for missing this. I may not have a chance to look at it until Monday.

louh commented 8 years ago

@meetar We might need to revisit the interface a bit. If I understand correctly, pushing this change out means that every current demo owner needs to add

MPZN.citysearch();
MPZN.geolocator();

to activate it at all.

I'm hoping to make the upgrade path as painless as possible -- especially since we ask demo owners to link to a centralized, unversioned library. This means that the moment we publish, we instantly remove city search and geolocator from every demo. And asking demo owners to put those two lines in first before we publish the library will cause its own problems, as those statements will currently throw syntax errors.

Furthermore, this:

MPZN.bug({
    bug(true)
});

is calling a function inside of an object, so I'm not sure what the intent was. bug: true ?

louh commented 8 years ago

We're going to migrate this work over to https://github.com/mapzen/scarab!