falling-fruit / falling-fruit-web

Mobile-friendly website for Falling Fruit
https://beta.fallingfruit.org
GNU General Public License v3.0
29 stars 7 forks source link

Move construction of types tree later in the flow #376

Closed wbazant closed 3 weeks ago

wbazant commented 1 month ago

When we do this given a selection and counts, we can make the tree look better by removing needless parents

New behaviour is to only keep the parents when there is branching under them Example 1: /map/@34.63293829116424,-81.5339308198741,9z + search for Actinidia the 'issai' cultivar appears in its own checkbox Screenshot from 2024-05-24 12-03-39

Example 2: zoom in on Glasgow and look for Wild garlic, then zoom out eventually it goes under 'Onion' during zooming out (together with Three-cornered leek annotation) Screenshot from 2024-05-24 12-04-32 Screenshot from 2024-05-24 12-04-44

Behaviour about selecting the parent, and parents being duplicated to also just stand for less specific annotations, is as before

The 'Pending Review' nodes still always go under their own node with the same behaviour

Also refactor to use different property names in tree-select ( previous choices reused other variables, relied on null stringifying to 'null', and added a 'root' to some variables and not others)

Also, don't put the null parent into result of getChildrenById

Closes #311

wbazant commented 1 month ago

I just noticed the change adds a warning in the console:

Warning: Conflict! value of node 'extra-2266' (2266) has already used by node '2266'.

Not sure what to do about it right now.

ezwelty commented 1 month ago

@wbazant Interesting idea. I agree this is mostly an improvement and happy to give it a shot. Where I hesitate is that the new version leads to a more variable tree structure as the selection changes. The same list of plant genus and a few non-botanicals (Freegan, Projects) at the top level is replaced by a child, depending on the selection. See for example (/map/@34.85118768141851,-82.35313392176981,13z):

More importantly, there seems to be a bug, as illustrated in this example. For example, Concord grape is at the top level despite being a grand-child of Grape. I suspect the grand-child replaced the parent, and then was no longer seen as related to its grandparent? Same for Peach and Santa Rosa plum (children of Stone fruit).

image

image

wbazant commented 1 month ago

I came to the conclusion that the idea of rewriting the tree structure wasn't the best, although like you say it's interesting. The modified PR addresses just #311 as originally described.

ezwelty commented 4 weeks ago

@wbazant More modest change but almost ready to go. Only issue I still see is that Pending Review shows "(NaN)" rather than "(0)" when count is zero.

wbazant commented 4 weeks ago

Thanks for catching that! I made a small change, now the count is calculated also for Pending Review IDs.

ezwelty commented 3 weeks ago

Closed! I removed your original mention of #353, since it isn't addressed by this PR.