search

LocalGround / localground

Other
18 stars 7 forks source link

Select map updates #162

Closed gitriley closed 6 years ago

vanwars commented 6 years ago

Setup Procedure

destroyed and recreated the database (started from scratch): 

$ vagrant ssh
$ DB_OWNER="postgres"
$ DB_NAME="lg_test_database"
$ HOST="localhost:7777"
$ psql -c "drop database $DB_NAME" -U $DB_OWNER
$ psql -c "create database $DB_NAME" -U $DB_OWNER
$ psql -c "update django_site set domain='$HOST', name='$HOST' where name = 'example.com'" -U $DB_OWNER -d $DB_NAME
$ cd /localground/apps
$ python manage.py migrate # builds the database according to the branch specs

Manual Testing Process

Step 1. I created a few Sites. I also create a custom site type called “Animals” with the following fields: (desc, species, and weight). After defining Animals, I created a few corresponding animal records. Step 2. I then went to the Style tab to create my first map. I clicked the “+” button, entered my first map name, and clicked save:

BUG 1: When I create my first map for a project and click “save” (/#/new), it create a new map, but then shows me the modal for a second time, encouraging me to either create another map or press cancel.

Step 3. I then deleted the auto-generated layer corresponding to my “Sites” data. I now only have one layer, called “Animals.” Step 4. I then edit the “Animals” layer and start changing the shape.

BUG 2: Instead of replacing the shapes on the map, it creates new shapes overtop of the old shapes (so each Animal instance has both a pin and a circle associate with it.

Step 5. I click the “Source Code” Tab.

BUG 3: It’s missing. In addition, I can’t get back to the visual editor again.

Step 6. I decide to start over by simply refreshing while the link is pointing to http://localhost:7777/style/#/1/layers/2. It opens to with the “Animals” layer detail panel opened, as expected. Nice. Now, when I change the symbols (Simple Marker Mode), it replaces them (instead of creating duplicate symbols). Perhaps Bug 2 only happens when the layers have first been created? Everything looks good now, however the Source Code button still doesn’t work.

Step 7. I edit the Map Description by clicking the pencil. Works as expected. Nice.

Step 8. I edit the fonts and colors on the left-hand panel, and click “Save Map.” I then click the eyeball preview button, and the presentation view comes up.

REQUESTS / THOUGHTS:

  • Can you add a cursor:pointer to the icons so the appear more clickable in the CSS?
  • What do you think about the green preview button? Should that button link just update every time the user moves between maps?

Step 9: Continuous Symbol Test. I add another layer by clicking the + button. I select “Animals” again as the datasource. I decide to make a continuous map according to each of my animal’s weights.

NICE: I really like that the layers w/no data are grayed out. NICE: Binned everything nicely. NICE: I could individually change the shapes and colors NICE: When I preview on the presentation map, all of my preferences persist. POINT OF CONFUSION: When I set individual properties, and then went back to adjust the ramp to a different set of colors, it overwrote all of my individual preferences. I would have expected it to only change the colors, but keep the shapes intact. Just wanted to make a note of it.

Step 10: Categorical Symbol Test: I add another layer by clicking the + button. I select “Animals” again as the datasource. This time, I want to make a categorical map according to “species” category (feline, k9, rodent):

NOTES: Everything works as expected. Nice.

Step 11: I create a second map, and try and toggle between the two maps.

NOTES: Works fine, but it’s not entirely clear to me when the layers default as on versus default as off.

Overall, nice work — just a few bugs to address.

Code Review

We’re still establishing a process for doing this more efficiently, so this checklist will be ongoing. But some basic ones:

General Testing Feedback

This has really come a long way, with a lot of usability enhancements and nice details / touches. Well done. Just a few bugs.

Going forward, I think what would have helped me be a more effective tester would have been to have a smaller PR. This PR was difficult to test, because I didn't really know exactly what I was supposed to be looking at, or what this PR was actually about. Just a note for the future: a list of things to test and an overall description of what this PR was about would be useful.

gitriley commented 6 years ago

Bug 1: Fixed this bug by routing to the newly created map upon 'save'. However, the route ultimately happens twice due to the following chain of events: 'change-map' event —> handleNewMap() —> 'ready-for-routing' event —> rerouteIfNeeded()

This doesn't cause any problems, it's just weird. By removing the 'ready-for-routing' event, the routing only happens once, without problem. However, I'm sure we have that code in there to handle some other issue, so I'll leave it for now.

Bug 2: I was unable to recreate this, but I can try again using your exact process, or you can show me on Thursday/Friday.

Bug 3: Fixed.

Step 9 POINT OF CONFUSION: Fixed. Changes to the color ramp will no longer overwrite individual shape modifications. They will override individual color modifications, obviously.

Step 11 Note Not sure exactly what you mean, but we can talk about it.

NOTE: I added some code so that the app will reroute correctly after maps and layers are deleted. e.g. if the url is: http://localhost:7777/style/#/277/layers/656

and the layer is deleted, it will reroute to: http://localhost:7777/style/#/277

If the map is deleted, it reroutes to http://localhost:7777/style/#

Final Note: I've noticed that the loading time when switching between maps is extremely slow. Do you experience this? I'm going to go back a few commits and find out if it's been this way for awhile or if it's being caused by some new code.