@spixi opened a PR about that and the discussion started on loomio was on a wrong thread so I’m opening this one.
Basically, this is about getting the location data from EXIF and then allow the user to display it on a map.
It has first been implemented on the
Notice the small marker at the bottom right of the picture. When you click on it, a map opens:
So the current problem with this design is that the map is really small. You can’t properly navigate in it so it becomes quite useless.
The proposition here would be to integrate this feature (a marker at the bottom right of the picture) inside the lightbox (the view displayed when a picture is clicked), as suggested by @denschub.
Here is a summary of how the feature can look like (feedback welcome if anybody has suggestions):
- The lightbox is completed to have info displayed on hover (Author, link to the post, location marker… see screenshot below)
- When clicking the location marker, the map is displayed in front of the picture
- The location marker can be kept on the
/photos page in my opinion as it is really small, but when it is clicked, the lightbox is open, with the map open (state described above) instead of trying to display a map in a really small tile.
What does everybody think about that?
Note: This discussion was imported from Loomio. Click here to view the original discussion.
Displaying the location data is one thing, but I feel saving it is much more important. I usually organise photos by place and time. The map was just an idea for the
/photos page. I have not implemented a map for the single photo view. I think, a small map in
/photos is fine, but the single photo view (which is shown, when you click on a photo in a status message) itself needs a larger map. This, however, requires a whole revision of that view. I also like to include additional information like title, description, date, hashtags, number of likes and reshares … The flickr view is a nice example for this. I don’t know if this is the right place to discuss this, because it mainly belongs to the discussion how to organise photos, but organising them geographically is one aspect and the map is a nice gimmick and it is consistent, because we already show a map for status messages.
I feel would be a great idea to add geo coordinates and a small map, which is already shown for status messages. Coordinates can retrieved from the Exif data of the pictures. This should be an optional feature. What do you think about this PR? https://github.com/diaspora/diaspora/pull/7272
Proposal: Add geo coordinates
I already tried to roughly implement this feature here: https://github.com/diaspora/diaspora/pull/7272
What do you think about it?
Note: This proposal was imported from Loomio. Vote details, some comments and metadata were not imported. Click here to view the proposal with all details on Loomio.
“What do you think about my rejected PR” is not a valid proposal. Not at all, in fact. Proposals are about implementing an idea or not. In any way, having a vote on this is pointless since a) there was no controversy b) we have not discussed about how we want to implement it. Voting on a subject that needs discussion is not constructive at all.
However, your pull request had nothing to do with this discussion at all. You implemented a simple map on top of the photos while this is discussion is about making the photo functionalities more rich. The PR got remarks regarding the presentation of the map data and implementation details, which you did neither address nor claim you’re going to address within almost two months of posting the remarks, which makes it pretty clear to us that you do not have interest in finishing the work, which is the only reason it was closed.
If you want to discuss on the possible UX for a map within the photos, then do it in a new thread. This is about albums or tags for photos to group them.
Maybe, Dennis, you try not to be too rude the newbies. Spixi did just like he/she was told to (except the opening of a voting).
But you’re right: this thread is about albums and not single-photos view. I am not sure if these two views are really two different things in this case. What we could need is a greater new concept for the whole photo-area. Unfortunately we don’t have this yet and noone knows if we will ever have one in the future.
For the meantime it could be helpful to find a compromise about this PR by Spixi. Maybe add just the location-icon to the view. And clicking it opens a dialog to show a larger and zoomable leaflet-map of the location.
I am totally happy about having a discussion on whether and how we could/should work on the photo section. There are some great remarks in both the PR and other discussions here, which surely can be worked on. However, as always with those ideas, someone actually would have to commit on working on implementing all the fancy stuff we have collected here…
you try not to be too rude the newbies. Spixi did just like he/she was told to
Except for the “annoying people on IRC and voice communication channels” parts, yeah, pretty much. And dropping another PR after discussions started. Sure thing!
@denschub I don’t know what you mean with “dropping another PR after discussions started”. Indeed I had another PR, but this was a fix to a script when you don’t have a global Bundler installation on your system. Further, I advise you to not mix-up the discussions happened on other channels with my work on the diaspora* project. Even when you dislike me, this is definitively not fair and I am sure, you are a clever guy and able to keep these different worlds apart.
I feel geo-coordinates are definitively a way to organise photos, e. g. it could be possible that you create virtual albums later with queries like “all photos taken in 2016 with the hashtag #cat shot in North-Rhine-Westphalia, Germany”. The map view was just an example use case without guarantee to be complete, as the pepper brush in GIMP is only an example use case for a pixmap brush. Without this view, my commit would make no sense at all.
But this is only a first step and as usual in iterative projects I am requesting for feedback, and when the majority of devs/users disagree with my proposal, then it is absolutely fine for me.
Once again, this is not about your pull request. There were reasons it got rejected and neither is this the right place to discuss that nor are we going to discuss that. We never merge “just an example use case” for the sake of merging it and this has been explained to you multiple times. It does not matter if people agree or disagree with your proposal since it is invalid. I am not going to quote myself, so you have to scroll up a bit for a long explanation about why the proposal is invalid.
So, I’m the one who redirected @spixi here, as my conclusion in the PR was “this feature has to be included in a more global thinking about photos inside diaspora*”.
Spixi is new here and was not aware of the rule “discuss without creating a vote” and that can be understood. Moreover, he was really quick to fix all remarks we did, the only one left I noticed is the federation part (not a small one I agree). I feel like the PR would not have been tagged orphan nor closed that quickly if it was on something which would have interest the core team. Sure, this feature will not be used by many people, but if it helps only a few, why not integrate it if the work is done? So in my opinion, what we needed was to discuss and find a good UI and Spixi looked ready to implement it. It’s to have that discussion, on a more global level, that I redirected him here. My bad if that hasn’t been understood.
That being said, I’m opening another discussion to see we can do about that as it looks like this is not the place to discuss about it.
Here is the link to the new discussion. Let’s not spam this thread anymore.
why not integrate it if the work is done?
I am honestly somewhat displeased by the fact I have to make the same statement over and over again, especially against a highly valued contributor like you, @flaburgan. Merging something merely because “someone has worked on it” is one of the worst reasons to merge something. In my very first non-reviewy comment in this PR, I clearly stated “some discussion is needed, and maybe even some mockups before implementing stuff”. That post got thumbs-up by @steffenvanbergerem and @supertux88, so we pretty much have an agreement by the entire active core team on that.
We’ve made that clear before and I am happy to do it again: Pull Requests with impact to the user interface need to be polished before getting merged or there needs to be a clear follow-up plan for polishing work ready before the PR is merged. Neither of those points were fulfilled. That’s a no-no for a merge.