BiG-CZ: List Popup #2314

arottersman · 2017-09-29T21:29:23Z

Overview

When BiG-CZ shapes overlapped we were only able to display the topmost layer. Add a popup that allows browsing details of all the shapes at the clicked point.

Connects #2282

Demo

Multipage

Single page

Notes

I left the height dynamic for now. When there's only a single item on a page, the above height "jumping" happens. I'm not sure what's desirable @jfrankl

Testing Instructions

Pull this branch and bundle
Spend some time searching and clicking the results of various catalogs
- If there's a single shape under your click, the details popup should show
- If multiple, the list popup should show
- You should still be able to highlight features by hovering over the sidebar items
- Searching, switching catalogs, and opening the details of a result should close the popup

rajadain

This works really well! The code is nice and neat, and the commit message packaging is done in a very accessible and easy to understand manner. Great job!

Regarding the height thing, I agree it is annoying and potentially disruptive, but at the very end of a page list. I think that's a small enough use case for us to not worry about at this time. The only design alternative I can think of is padding the results, but the whitespace may be more awkward than the height change.

Here are a few issues I noticed though:

Hovering on items in the popup list does not highlight them on the map or the sidebar. It should, since it all other interactions work the same.
Clicking on item in the list and then reaching the detail popup does not highlight the item on the map or the sidebar. It should.
Hovering on an item in the sidebar hides the popup. This should not happen. Clicking should hide it, but hovering should not.
In many cases, the title is not useful enough to differentiate in the first few characters that are shown for each item. I wonder if we should have two lines for the titles before (dot dot dot)ing them. This could use the input of @ajrobbins and @jfrankl

I'll take another look once 1, 2, 3 above have been addressed.

rajadain · 2017-10-02T15:05:06Z

src/mmw/js/src/core/views.js

+                    },
+
+                    // Get a list of results intersecting the clicked point
+                    intersectingFeatures = _.reduce(


Prefer using _.filter instead of _.reduce:

intersectingFeatures = _.filter(e.target._layers, intersectsClickBounds);

rajadain · 2017-10-02T15:05:55Z

src/mmw/js/src/core/views.js

+                    layer.setStyle(dataCatalogActiveStyle);
+                    result.set('active', true);
+
+                    layer.on('popupclose', function() {


Is this event handler taken off at any point? Or do they just pile on with more and more clicks?

They were probably piling, good catch. I'm changing this to once

rajadain · 2017-10-02T15:42:48Z

Great job on attaching demo gifs for every use case too 👌

ajrobbins · 2017-10-02T15:49:04Z

In many cases, the title is not useful enough to differentiate in the first few characters that are shown for each item. I wonder if we should have two lines for the titles before (dot dot dot)ing them. This could use the input of @ajrobbins and @jfrankl

I think the existing treatment is sufficient, since we don't know what format the title will take (the examples shown here are only one kind of result)

rajadain · 2017-10-02T15:51:14Z

This is one example of important information being cutoff:

the first result is for Maryland, the second for Pennsylvania, but that's hard to make out.

ajrobbins · 2017-10-02T16:00:03Z

That's a good example of it not accommodating important information. However, the same could be true if we have two lines, since there is no standardization on the result titles coming in from the API.

arottersman · 2017-10-03T22:57:09Z

Thanks, @rajadain. Those were good points. I've pushed some additional fixups + commits to address. Properly linking the popup state and map + sidebar was unexpectedly difficult, but I think it's working now (issue described in commit message).

rajadain · 2017-10-04T18:04:57Z

Taking another look at this.

rajadain · 2017-10-04T18:25:04Z

With changes in the latest commit, there seems to be a regression: if a CINERGI shape is too big for the map, it's gold border (which should convert to regular blue) does not "stick" as it should when the details view is open:

rajadain · 2017-10-04T18:28:14Z

Another one: if a popup is open and its corresponding sidebar element is (correctly) highlighted, and I zoom the map, the popup closes but the highlight gets stuck, and then can't be unhighlighted:

rajadain · 2017-10-04T18:31:22Z

For WDC, if a popup is open for a single element, hovering on others keeps the yellow highlight of the popup-element there. That is, there are two highlighted elements on the map: one for the popup and another for the sidebar element on which the user is hovering.

For the multi-popups, this does not happen. Hovering on any element in the sidebar removes the highlight of the popup element. This should not happen:

rajadain · 2017-10-04T18:31:52Z

I'll take another look once these issues have been addressed.

arottersman · 2017-10-05T13:04:45Z

Will push a fix soon for the 3rd comment about WDC. I'm having trouble recreating the other two issues described.

With changes in the latest commit, there seems to be a regression: if a CINERGI shape is too big for the map, it's gold border (which should convert to regular blue) does not "stick" as it should when the details view is open:

Here's the blue border I see on the details view:

Another one: if a popup is open and its corresponding sidebar element is (correctly) highlighted, and I zoom the map, the popup closes but the highlight gets stuck, and then can't be unhighlighted

Here's what happens when I zoom

arottersman · 2017-10-05T17:51:14Z

@rajadain and I looked together IRL, and he wasn't making up those bugs!

I've squashed down the fixups and added a commit that adds a popupclose listener for the list popup. I think it's mostly fixed the first two issues. The third issue will require a greater refactor; I'll write up a separate card.

Here's a brief recap

With changes in the latest commit, there seems to be a regression: if a CINERGI shape is too big for the map, it's gold border (which should convert to regular blue) does not "stick" as it should when the details view is open

This was recreatable by going to the details view from the list, and then hovering over the map (the highlight would disappear). The popupclose commit seems to have fixed this, though the detail highlight still doesn't show up on FF until you've panned/zoomed the map. I'll make a card.

EDIT: @rajadain showed me this did not fix it. Making an issue

Another one: if a popup is open and its corresponding sidebar element is (correctly) highlighted, and I zoom the map, the popup closes but the highlight gets stuck, and then can't be unhighlighted

We were also able to recreate this together by going from list popup -> detail popup, then closing the popup. I believe it's fixed now.

For WDC, if a popup is open for a single element, hovering on others keeps the yellow highlight of the popup-element there. That is, there are two highlighted elements on the map: one for the popup and another for the sidebar element on which the user is hovering.

For the multi-popups, this does not happen.

This is the one I'll be making an issue for. We currently set the active styling directly on the popup layer, instead of using the map state. We can't do this for the list popup because the popup is on the map, not a specific layer.

arottersman · 2017-10-06T15:38:27Z

@rajadain, is it worthwhile to rebase this on develop?

rajadain · 2017-10-06T15:41:31Z

In the interest of not adding things to the release/1.20.0 branch that don't belong there, I would say yes, we should rebase and retarget to develop. Thanks for catching that

* Previously we didn't fill the Cinergi boxes, so clicks only registered on their borders * Transparently fill the boxes so we can capture all intersecting clicks

* Previously only the topmost catalog feature would fire a click event * We want to show all intersected layers per click, move the popup binding into an on click handler on the whole FeatureGroup * Clicks that only intersect a single feature show popups the same as before * Clicks that intersect multiple features, just log them to the console for now

* Create a new popup view that first shows a list view of all click-intersected results. If a list item is clicked, shows the selected result's detail view

* The list popups are directly on the map, not tied to any particular layer. Close them manually when the catalog is switched or we enter a detail view

* Use Backbone Pageable Collection to paginate popover list view when results > 3 items

* Highlight the sidebar list item and map element when hovering a popup list * This involved changing the way we add the "ring" highlight to the map when the shape is outside the viewport bounds for CINERGI - Before we were using jquery to add a "highlight" class to the map container. This caused additional mouseenter and mouseleave events to fire on the popup list element. The cursor and highlighting would flicker - Switch to adding a whole DOM element to show the "highlight" ring. This does not cause additional mouse events to fire.

arottersman · 2017-10-06T16:35:41Z

Rebased on develop, and updated target.

rajadain · 2017-10-06T17:18:40Z

Waiting on #2351 to fix the build, after which this should pass ...

rajadain · 2017-10-06T19:33:09Z

@azavea-bot rebuild

rajadain · 2017-10-06T20:40:10Z

Taking another look to confirm

rajadain

+1 tested. This works well, and we have follow up issues #2335 and #2336 for the rest.

Great work translating the designs and nailing the execution.

arottersman · 2017-10-06T21:39:31Z

Thanks for all your help with this, and uncovering those bugs.

arottersman added the in progress label Sep 29, 2017

arottersman requested a review from rajadain October 2, 2017 14:08

arottersman assigned rajadain Oct 2, 2017

arottersman changed the title ~~WIP Arr/bigcz layered popup~~ BiG-CZ: List Popup Oct 2, 2017

rajadain requested changes Oct 2, 2017

View reviewed changes

rajadain added the BigCZ label Oct 2, 2017

arottersman force-pushed the arr/bigcz-layered-popup branch from b666014 to f5d20d2 Compare October 5, 2017 17:38

This was referenced Oct 5, 2017

List Popup Highlighting Improvements #2335

Open

Detail view doesn't show cinergi blue map halo until map has updated #2336

Closed

arottersman force-pushed the arr/bigcz-layered-popup branch from 18ba277 to 4ee102e Compare October 6, 2017 16:29

arottersman changed the base branch from release/1.20.0 to develop October 6, 2017 16:30

Alice Rottersman added 5 commits October 6, 2017 12:32

BiG-CZ: Register Clicks Inside Cinergi Bounding Boxes

462d348

* Previously we didn't fill the Cinergi boxes, so clicks only registered on their borders * Transparently fill the boxes so we can capture all intersecting clicks

BiG-CZ: Render List Popup View On Multiple Features Click

b398ac3

* Create a new popup view that first shows a list view of all click-intersected results. If a list item is clicked, shows the selected result's detail view

BiG-CZ: Add number of results to popover list view

e141c73

BiG-CZ: Add back button from details popover to list popover

0139cd2

Alice Rottersman added 4 commits October 6, 2017 12:34

BiG-CZ: Close popovers when switching catalog/detail view

88f8caf

* The list popups are directly on the map, not tied to any particular layer. Close them manually when the catalog is switched or we enter a detail view

BiG-CZ: Paginate Popover List View

7e49722

* Use Backbone Pageable Collection to paginate popover list view when results > 3 items

BiG-CZ: On list popup close, clear active styling

9bb0e53

arottersman force-pushed the arr/bigcz-layered-popup branch from 4ee102e to 9bb0e53 Compare October 6, 2017 16:34

This was referenced Oct 6, 2017

Overlapping WDC Results #2210

Closed

Consider sorting results by geographic relevancy #2115

Open

rajadain approved these changes Oct 6, 2017

View reviewed changes

rajadain assigned arottersman and unassigned rajadain Oct 6, 2017

arottersman merged commit fbff38a into develop Oct 6, 2017

hectcastro removed the in progress label Oct 6, 2017

arottersman deleted the arr/bigcz-layered-popup branch October 6, 2017 21:39

rajadain mentioned this pull request Nov 10, 2017

Make Popups Show Contents of All Visible Layers rajadain/trelliscope#11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BiG-CZ: List Popup #2314

BiG-CZ: List Popup #2314

arottersman commented Sep 29, 2017 •

edited

Loading

rajadain left a comment

rajadain Oct 2, 2017

rajadain Oct 2, 2017

arottersman Oct 3, 2017

rajadain commented Oct 2, 2017

ajrobbins commented Oct 2, 2017

rajadain commented Oct 2, 2017

ajrobbins commented Oct 2, 2017

arottersman commented Oct 3, 2017

rajadain commented Oct 4, 2017

rajadain commented Oct 4, 2017

rajadain commented Oct 4, 2017

rajadain commented Oct 4, 2017

rajadain commented Oct 4, 2017

arottersman commented Oct 5, 2017

arottersman commented Oct 5, 2017 •

edited

Loading

arottersman commented Oct 6, 2017

rajadain commented Oct 6, 2017

arottersman commented Oct 6, 2017

rajadain commented Oct 6, 2017

rajadain commented Oct 6, 2017

rajadain commented Oct 6, 2017

rajadain left a comment •

edited

Loading

arottersman commented Oct 6, 2017

BiG-CZ: List Popup #2314

BiG-CZ: List Popup #2314

Conversation

arottersman commented Sep 29, 2017 • edited Loading

Overview

Demo

Notes

Testing Instructions

rajadain left a comment

Choose a reason for hiding this comment

rajadain Oct 2, 2017

Choose a reason for hiding this comment

rajadain Oct 2, 2017

Choose a reason for hiding this comment

arottersman Oct 3, 2017

Choose a reason for hiding this comment

rajadain commented Oct 2, 2017

ajrobbins commented Oct 2, 2017

rajadain commented Oct 2, 2017

ajrobbins commented Oct 2, 2017

arottersman commented Oct 3, 2017

rajadain commented Oct 4, 2017

rajadain commented Oct 4, 2017

rajadain commented Oct 4, 2017

rajadain commented Oct 4, 2017

rajadain commented Oct 4, 2017

arottersman commented Oct 5, 2017

arottersman commented Oct 5, 2017 • edited Loading

arottersman commented Oct 6, 2017

rajadain commented Oct 6, 2017

arottersman commented Oct 6, 2017

rajadain commented Oct 6, 2017

rajadain commented Oct 6, 2017

rajadain commented Oct 6, 2017

rajadain left a comment • edited Loading

Choose a reason for hiding this comment

arottersman commented Oct 6, 2017

arottersman commented Sep 29, 2017 •

edited

Loading

arottersman commented Oct 5, 2017 •

edited

Loading

rajadain left a comment •

edited

Loading