CityScope / CS_cityscopeJS

Create, design, and publish @CityScope projects on the web
https://cityscope.media.mit.edu/CS_cityscopeJS/
GNU General Public License v3.0
20 stars 16 forks source link

Some random grid cells can't be edited #64

Closed doorleyr closed 4 years ago

doorleyr commented 4 years ago

Interacting with the Corktown grid, there seems to be some grid cells which I'm unable to edit. These cells seem to random. See attachment which shows a vertical line of cells and a horizontal line of cells which I can't edit.

Screenshot 2020-03-19 at 17 16 33
RELNO commented 4 years ago

Screen Shot 2020-03-19 at 13 32 58

Looks to me like they follow your non-editable roads?

doorleyr commented 4 years ago

It is being driven by the GEOGRID alright but on closer inspection, it's not being driven by the right property. Instead of being driven by the interactive property, it is making any cell with 'land_use' of None as non-interactive. I'm attaching two screenshots from QGIS: the first is highlighting the cells with interactive=True (all the cells that should be interactive) and the other is highlighting the cells with land_use=None

Screenshot 2020-03-19 at 19 14 33 Screenshot 2020-03-19 at 19 41 25

The FE should be updated such that only cells with interactive=True can be edited.

RELNO commented 4 years ago

Yes, that's the case. It's a by product of the way we did it for GB, and the lack of standardization in land uses. In other words, there should not be a case of no-land use and we should only use mapping to trigger interactivity. I can fix it locally but this pings back to @LAAP and Markus work.

doorleyr commented 4 years ago

I can update the Grasbrook GEOGRID so that all of the cells which are supposed to be non-interactive have the property interactive=False. Then if you change the FE to use this criteria instead, we're done. Even if the mapping were ready, I think the above is the right way to do it, right? I can't see how we could continue to have interactivity driven by the baseline land uses. For example in Corktown, the areas which should be interactive has nothing to do with baseline land uses- it needs to be specified separately: that's why we have the interactive flag.

RELNO commented 4 years ago

is this still open or will it be solved with #57 ?

doorleyr commented 4 years ago

I won't be solved by #57. As I outlined above, the interactivity needs to be driven by the 'interactive' flag.

RELNO commented 4 years ago

OK -- currently, the lack of standard data type forces me to render cells per their type (if exist or not). The interactive mapping is also used. As soon as we agree on types standard, we can also agree on the format of the mapping object https://github.com/CityScope/CS_cityscopeJS/blob/master/docs/schema.md

doorleyr commented 4 years ago

There's no need to rely on standards or interactive mappings if we're using the interactive flag as the condition. The interactivity of cells right now is governed by an if statement on line 309 of https://github.com/CityScope/CS_cityscopeJS/blob/master/src/components/baseMap/baseMap.js:

if ( this.props.selectedType.class === "buildingsClass" && thisCellProps && thisCellProps.land_use !== "None" )

In my local, I just replaced this with: if ( this.props.selectedType.class === "buildingsClass" && thisCellProps && thisCellProps.interactive == true )

and it works perfectly for Corktown and Grasbrook. To make it work for any other table we just need to make sure the the interactive flag is set correctly in GEOGRID and inherited by GEOGRIDDATA.

RELNO commented 4 years ago

@doorleyr true, it's a residue from early version that used non types as flags. Care to PR your change?

doorleyr commented 4 years ago

Sure https://github.com/CityScope/CS_cityscopeJS/pull/68

RELNO commented 4 years ago

closing this given today GH-pages deployment