theCrag / website

theCrag.com: Add your voice and help guide the development of the world's largest collaborative rock climbing & bouldering platform
https://www.thecrag.com/
110 stars 8 forks source link

Link to annotations from topos same as areas #1693

Closed brendanheywood closed 5 years ago

brendanheywood commented 9 years ago

Now that annotations have title it make a whole lot more sense to be able to link to them in an identical fashion to an area. Almost zero code from the topos point of view, just need to make sure we pass from the annotation id and it's parents id to handle the case where it is linked from outside the parent area.

Must do Tasks:

Nice to have:

Tests:

brendanheywood commented 9 years ago

@tomkjaerolsen

brendanheywood commented 9 years ago

There is one small complication in that an annotation could be added to a topo from a different area, so we need to link to the annotations parents url with the annotations hash anchor. Might need a little more template data for this, need to check

brendanheywood commented 8 years ago

Got a perfect test case for this:

https://www.thecrag.com/climbing/australia/gara-gorge/upper-gara-gorge/the-fortress

brendanheywood commented 7 years ago

+1 from @rouletout for this

As an example I would like to draw 3 sub-sectors of this crag https://www.thecrag.com/climbing/united-states/homestead/area/1049626368 The sub-sectors are just separated by Annotations but they are not available for topo drawings.

rouletout commented 6 years ago

+1 again, I can't do what I want if I can't draw annotations in an area topo. Is that hard to add?

brendanheywood commented 6 years ago

@rouletout as you find example where you want it can you also add the urls to this issue so that when we implement this we can go back and fix them up as good examples to show case.

There is probably a days work for me here and work for @scd too, I'd guess max a day on his side

rouletout commented 6 years ago

examples are / will be here:

https://www.thecrag.com/climbing/united-states/colorado-shelf-road/area/16741327 https://www.thecrag.com/climbing/united-states/colorado-shelf-road/area/14385913 and its sub-areas set up as sectors

brendanheywood commented 5 years ago

+1 from private email

brendanheywood commented 5 years ago

+1 from gym

scd commented 5 years ago

@brendanheywood I have got some way with this, but I think we need to make some minor enhancements with the topo tool event handler.

THe annotation is in the list of topo objects but when I click on the annotation line the topo stops working. I suspect it is something to do with data-type="annotation" not being recognised. Is this something you can look into

In branch scd/topo-annotation

brendanheywood commented 5 years ago

I've found a few hours and done the bulk of the topo js work now:

https://brendan.thecrag.com/climbing/australia/gara-gorge/upper-gara-gorge/the-fortress#contents

@scd I've added just the parentID to the annotation atom which is enough for anchor linking within the same page. But for the edge case where you link to an annotation on a different node, can you please add template data to the annotation atom for it's parents node details, just enough to build a url. If you prefer, please only do this is the annotations parent is different to the topos base node if you want to same data.

scd commented 5 years ago

this is great, i have done the parent

brendanheywood commented 5 years ago

We need to make a couple business decisions to close this one off:

1) numbering policy: ie do we include annotations in the numbering order. We don't need them to be included for any particular reason even if they are in the topos, so I think we should leave them as they were. Was there a reason this was changed?

2) annotation locations: I've temporarily commented out the new code which exposes the location or lack of a location of an annotation. This creates a large dependency on more changes to the maps code which we don't need to do now so I think this is already decided for us.

I've cleaned up all the remaining relative vs absolute anchor bugs in all the different topo contexts, so @scd if you can sort the number stuff then I think we are done

scd commented 5 years ago

@brendanheywood this issue is now sorted as far as I am concerned. Please review and close.

final changes were:

brendanheywood commented 5 years ago

Looks good, small task for myself:

The use of the route template looks great, we should migrate to that across the board in all the processes