Skip to content

Conversation

@gentlegiantJGC
Copy link
Contributor

@gentlegiantJGC gentlegiantJGC commented May 12, 2022

This adds a context menu item to reposition pins.
When the context menu item is clicked the next click on the map will move the pin to that location.

You might want this implemented differently. It does not currently have any user feedback but it functionally works.

I still need to implement support for changing the front matter attribute.

Added an attribute to the FileMarker class to store the inline match length
Added a context menu item to the map pin to edit location which stores the pin attribute
When the map is clicked and the pin attribute is not null it will rewrite the pin location with the clicked location.
Implemented changing inline locations.
Implemented changing front matter locations.

resolves #43

Added an attribute to the FileMarker class to store the inline match length
Added a context menu item to the map pin to edit location which stores the pin attribute
When the map is clicked and the pin attribute is not null it will rewrite the pin location with the clicked location.
So far implemented inline locations.
Still need to implement front matter locations.
@esm7
Copy link
Owner

esm7 commented May 13, 2022

That's a great addition that I think many users will greatly appreciate!
I think it should be implemented a bit differently though. Leaflet.js supports a draggable option to Marker (see https://leafletjs.com/reference.html#marker-draggable). I suggest that in a marker context menu there would be a "Move Location" entry (similar to what you did), which will set that marker to draggable. Then change that marker color to look different until the user chooses "End Move" or clicks it.
You will then start to get dragend events, on which it will be the place to modify the note content, similarly to what you started.

@gentlegiantJGC
Copy link
Contributor Author

Option 1 is the pin is always draggable. I don't think this should be the default behaviour but there should be an option to enable this behaviour to make it easier to reposition a lot of points. Perhaps in the map context menu.

Option 2 is kind of jank but is the best way I can see to do it currently. Right click on the pin which makes it draggable via marker.dragging.enable() then the user can drag it where they want (but requires them to initiate the drag) then when finished dragging set it to be not draggable.

Ideally when the context menu is clicked it should get picked up and follow the mouse but I can't see a way to initiate this without the user doing it.

@esm7
Copy link
Owner

esm7 commented May 13, 2022

I think your point of making it easier to reposition many markers is important.
How about adding a map control (e.g. below the search tool) that will turn repositioning on and off?

@gentlegiantJGC
Copy link
Contributor Author

Yeh I could add it to the menu. I was trying not to clutter it too much.
I would still like to try and support editing individual markers.

Is there a way to trigger the drag start without the user doing it?
I can fire an event but I don't know if this just runs the handlers or actually runs the action.

@esm7
Copy link
Owner

esm7 commented May 13, 2022

You can probably start it by sending a mousedown event, but why do that?
If unlocking the move ability is a map tool, and when it's on you set all markers to draggable, I think just dragging markers around would give the most natural user experience.

@gentlegiantJGC
Copy link
Contributor Author

I think you are right. I tried firing the mousedown event but it did not do anything.
This would also only work on desktop. On touch that logic would break but there is no mouse to follow.
I will just at a toggle to the menu

@gentlegiantJGC
Copy link
Contributor Author

I have this mostly working.
I would like to suppress/hide the preview window when moving the pin because it gets in the way.
How do I do this?
It looks like you trigger it using link-hover but I can't see any documentation on this.
Calling newMarker.closePopup() hides the name popup but not the preview popup.

Switched from the context menu implementation to using Leaflet's dragging mechanism.
Editing is toggled from the check box in the left menu.
@gentlegiantJGC
Copy link
Contributor Author

Clearly I don't know enough to hide that popup.
I have tried a number of things but I can't work out how to fire any events properly. Nothing I have tried causes the handlers to run.
I will finish up the coord modification and hopefully you can work out how to hide it.

@esm7
Copy link
Owner

esm7 commented May 14, 2022

If you get the big things working (most notably a map control that toggles with an indication when we're in move mode, and the function that reliably edits notes' locations in all cases), I can sure take care of that popup!

@gentlegiantJGC
Copy link
Contributor Author

gentlegiantJGC commented May 14, 2022

I am trying to get yawn-yaml to work so that I can load, edit and write back the yaml front matter.
For some reason it is not working. It looks like for some reason the dependencies of yawn-yaml are not being included and are undefined when running.
Any idea what I can do so that they are included?

There may be bugs in these because I cannot package them correctly to test.
@gentlegiantJGC
Copy link
Contributor Author

@esm7 I think that is mostly there.
For some reason I can't get yawn-yaml to work.
I think the issue is that the dependency lodash is not getting included. All attributes of the module are undefined.
This may be because it is a commonjs library but I don't know much of the javascript ecosystem.

You can recreate this by making a marker then checking the Edit Markers button and dragging a marker.
Upon releasing the marker, the error shows in the console.

Switched yaml handling to the parseYaml and stringifyYaml functions from the obsidian API.
They reformat the yaml so it is not ideal but does functionally work.
@gentlegiantJGC
Copy link
Contributor Author

I have switched to the parseYaml and stringifyYaml functions which work but reformat the yaml, remove comments and replace empty values with null so it isn't an ideal solution but it does at least work.
If we can get yawn-yaml to work and it doesn't have any bugs it would be the better solution.

frontMatterYaml already ends in a newline so adding another one leads to a blank line
On windows the end of line can be \r\n (the \r is optional)
Make yaml match lazily so if the front matter is duplicated it only matches the first one.
@esm7
Copy link
Owner

esm7 commented May 16, 2022

We can't have Map View change the users' front matter, gotta resolve this before moving on :-/

  1. What about js-yaml?
  2. How about going much more lightweight and just editing the front matter in-place with a regex, using the original geolocation string as the source to replace? If you only replace [x, y] by [a, b], only in the original file location that [x, y] was found, and fail if an exact match for [x, y] is not found, maybe we shouldn't care about parsing the YAML at all.

@gentlegiantJGC
Copy link
Contributor Author

The front matter after loading is identical however the formatting is changed.
Yaml lists can be split over multiple lines so it would be a rather complex regex. I don't know if it is actually possible because of the issue I listed in #84

I think js-yaml works the same as the functions in obsidian.
yawn-yaml should keep the formatting but I can't get it to work

@esm7
Copy link
Owner

esm7 commented May 17, 2022

IMHO it is perfectly fine to support moving of markers only if they are formatted in the two simple single-line formats that Map View uses.
If a user formatted his own YAML to split the location to two lines or something along this line, moving a marker should fail and it's a very reasonable limitation of the feature.

There can be whitespace before the first --- in the yaml
@gentlegiantJGC
Copy link
Contributor Author

The one line format is a subset of the full yaml format.
The following is still valid yaml and the pin will be displayed but will not be editable which will confuse users (or will be displayed and the user can drag it to a new location but the changes will not get saved to the file and their changes will not persist)

---
location:
  - 51.50852314163118
  - -0.07697738707065582
---

@esm7
Copy link
Owner

esm7 commented May 17, 2022

I think it's all a matter of error handling.
If in the handling of dragend you'd reload the relevant marker, and it will be shown correctly if it was updated or jump back to the previous location if it was not, I think it's a perfectly fine implementation.

@gentlegiantJGC
Copy link
Contributor Author

I don't particularly like the solution.
Did you try getting yawn-yaml to work?

@esm7
Copy link
Owner

esm7 commented May 17, 2022

Like you wrote, there is some issue there with the dependencies, but when trying to dig deeper I suddenly realized that this attempt to get the YAML parsing perfect seems to me like over-engineering it...
Map View writes YAML locations in one single way, very few users (if any) write YAML locations by hand (why do that? Map View offers so many ways to do that for you). If any of them happens to use a different YAML format for some reason -- I don't see this as a justification to add more libraries with big chunks of code to the plugin...

@bryanwhiting
Copy link

A year later, I'd love to see this merged! :) Would love to drag and drop markers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Would be great if I could reposition pins (maybe right click and select "move pin")

3 participants