-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Places: Add cluster preview resize handle #3888
base: develop
Are you sure you want to change the base?
Places: Add cluster preview resize handle #3888
Conversation
Awesome! We'll review this as soon as possible and then merge it if there are no issues found 👍 |
@armribdev As I understand your changes, the overlay now stays open when you click on another cluster? I believe it currently closes briefly and then opens again, so that's great :) Once you've signed the Contributor Agreement, we would love to proceed with testing so that we can merge this. Note that with our CLA, you retain all rights, title and interest in your contributions. However, we prefer a formal contract to avoid legal uncertainties for you and us, especially since we receive contributions from countries around the world with different legal systems and copyright standards. |
@lastzero yes, it was glitching a bit. And with css transitions it's now way more smouth :) |
Thank you very much for working on this! It works very well on desktop (tested in different browsers on Linux, macOS and Windows) 👍 On mobile devices I have observed two things:
I've attached a screen recording that shows both behaviors. resize-handle-recording.mp4@lastzero @armribdev Any ideas if/how we can improve this? |
Doesn't this collide with the waiving of rights you explicitly agree to there? I understand that you would ALWAYS retain the "Urheberrecht" since the jurisdiction of Germany doesn't enable you to waive that, but you certainly seem to waive SOMETHING here...? |
@GlassedSilver Here is a summary of what our CLA does (or at least is supposed to do), with explanations: (1) You GRANT us the right to use the code - and any patents attached to it - for the project (in any way we want, not just for Purpose A or under the current license, so we can never change it again). That means you can't just give us the code and then charge for the patents once we've merged it. (2) You also sign that this is your work and not a copy of someone else's copyrighted work. (3) In addition, the CLA confirms that your contribution is "as-is" and that you have no obligations, such as providing support to our users or assuming liability if there are security issues and, for example, our users' private pictures become public as a result. You certainly do not want to be liable for that when you give something "for free", i.e. without strings attached, see (1). With regard to (3), note that we give a free membership to regular contributors and whatever else we can afford. So legally speaking, one COULD argue that you are being paid and therefore there MIGHT be liability, depending on the legal framework, which is why the CLA also defines which law applies. Does that make sense to you? |
It does indeed, thanks for clarifying! I speak from multiple firsthand experiences and am currently in the process of moving a lot of older photos and videos in my library away from Aperture, which is an awful experience to say the least if you also used its non-destructive editing. I have since conceded I'll never keep my adjustments editable and just export TIFFs of edited versions, but I learned a valuable lesson when Apple killed Aperture, so naturally I'm HIGHLY cautious about what I use for my photos going forward. I've always enjoyed Photoprism - for the record - and absolutely love that so much focus is put on UX as well as technical prowess and format support. The fact that Photoprism is a German FOSS project just adds to it. :) |
@GlassedSilver Thanks for asking! I've updated the related section in our Open Source FAQ, as the previous explanation there may not have been clear enough: https://www.photoprism.app/oss/faq#cla |
This pull request implements a handle to the cluster view in places as asked in issue #3747. It's compatible with touchscreens with a larger touch area to be easier to use.
I didn't add tests as I don't think it's necessary for this.
Acceptance Criteria: