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

Adds a title in bubbles with items to show more info about the place #144

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lmorillas
Copy link
Contributor

When bubbles for multiple items are created don't show info about their place.
This feature ads a new tag when map view is created.

bubblewithitems

@karger
Copy link
Member

karger commented Dec 6, 2015

Louis, I'm afraid that without usage instructions and an example of usage it's a little bit hard to figure out exactly what this change does.

Also, if you're going to clean up whitespace (generally a good thing) please do so in a separate commit. It's hard to see/understand the "real" changes when the diff is cluttered with lots of whitespace changes.

@lmorillas
Copy link
Contributor Author

Ok, David. I'll write some documentation because I think it's useful.

@lmorillas
Copy link
Contributor Author

There is a new attribute in the div view of Exhibit Maps: data-ex-marker-label-point This keys is used in the bubbles for multiple items. When there is more than one item in a buttle, it show a label for all of them.

<div data-ex-role="view" 
    data-ex-view-class="Map" 
...
    data-ex-marker-label-point=".label_for_items"
...
>

@karger
Copy link
Member

karger commented Dec 15, 2015

Louis, a few things I'd like to address. First, your pull request has 4 commits, but they aren't a sequence. The third commit has the same parent as the first commit (as well as the same message). And the 4th merge commit has no changes at all. So I'm not sure why all 4 of them are present.

Also, as I mentioned above, there's a small amount of code change and a lot of whitespace change. The whitespace change makes it hard to view/assess the code change. If you can make one commit with only whitespace changes and another with only code changes, it's a lot easier to assess the pull request.

@karger
Copy link
Member

karger commented Dec 15, 2015

And here's a separate, design question. Your approach to picking a "header" label is to take evaluate the header expression on the first item in the list. This means that every item needs to be given a header in case it becomes the first item in the list after filtering, which is a lot of redundancy. And it seems a little arbitrary to pick from the first item. I still haven't seen a working example so I'm not sure exactly what kind of header is wanted.

As alternative, a header that could be provided without any additional data would be the kind we put on a list view, namely "5 Items" (or whatever "type" is associated with the items in the collection). Would this meet the need?

If you actually want a place-specific label, is it the individual items that should be contributing that label, or is it the place that should get a label that is used whichever items from that place are displayed in the bubble after filtering? Would a "Place Coder" that works like the current color coder, that associates a label with specific geographic coordinates, suit this need better? Or is it easier to have the place labels in the data file?

@lmorillas
Copy link
Contributor Author

This is our working example http://area47mil.educa.aragon.es/visual2/mapa_innovacion.html right now. A place coder could be interesting. But the easiest way is to have the place labels in a data file.

@karger
Copy link
Member

karger commented Dec 27, 2015

Your usage demonstrates an interesting and potentially problematic redundancy in the current approach. Your map view specifies both a latlng and a marker-label-point. The latlng specifies the location of the item, and then the marker-label is "transfered" from the item to the location it is associated with.

But really the marker-label belongs with the location, not with the item. If you associated the label with the location, then instead of having to give a marker-label value to every single item (one value per item) you would only need to give a marker-label value to each location (one value per location). That removes some redundancy which is a good thing---having the redundant values means someone at some point will fail to update one of the items' marker-labels and you'll get a label that changes based on which items have been filtered out of the view.

It seems that a cleaner approach here would be to define a collection of locations, each of which has a latlng and also a label and perhaps other properties. Then, for items you want to plot on the map, instead of directly specifying its latlng, you specify its location. Now the redundancy is gone: locations have latlngs (and labels), items have (only) locations.

This opens some interesting possibilities for the future. For example, the map lens might be able to refer to both data in the location object and the various items associated with it. One could even have a location lens separate from the item lens; the location lens is what opens up on the marker (and gives the list of items) while the item lens presents individual items.

The implementation of this is not entirely trivial since it requires adding a location accessor to the map view; I think in many ways this would be similar to the "proxy" accessor in the timeline view.

Would you be interested in this approach?

@lmorillas
Copy link
Contributor Author

Cool, David. This is a very interesting approach. I can work on it this week.

@lmorillas
Copy link
Contributor Author

The last approach you suggest is very interesting. Tell me if you want me collaborate with you with some code or if you are thinking on some kind of solution.

Data in my case hasn't redundancy because latlng and marker-label-point are attributes of items of the school's collection. In my case resources (videos) are associated with schools. I choose the first marker-label because all of the resources of a place belong to the same school (share geolocation, name and other attributes)

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

Successfully merging this pull request may close these issues.

2 participants