Skip to content
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

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

armribdev
Copy link

@armribdev armribdev commented Nov 15, 2023

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:

  • Features and enhancements must be fully implemented so that they can be released at any time without additional work
  • Automated unit and/or acceptance tests are mandatory to ensure the changes work as expected and to reduce repetitive manual work
  • Frontend components must be responsive to work and look properly on phones, tablets, and desktop computers; you must have tested them on all major browsers and different devices
  • Documentation and translation updates should be provided if needed
  • In case you submit database-related changes, they must be tested and compatible with SQLite 3 and MariaDB 10.5.12+

@CLAassistant
Copy link

CLAassistant commented Nov 15, 2023

CLA assistant check
All committers have signed the CLA.

@lastzero
Copy link
Member

Awesome! We'll review this as soon as possible and then merge it if there are no issues found 👍

@lastzero
Copy link
Member

@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 lastzero added waiting Impediment / blocked / waiting enhancement Refactoring, improvement or maintenance task ux Impacts User Experience labels Nov 16, 2023
@lastzero lastzero changed the title Feature/places cluster preview resize handle Places: Add cluster preview resize handle Nov 16, 2023
@armribdev
Copy link
Author

@lastzero yes, it was glitching a bit. And with css transitions it's now way more smouth :)

@lastzero lastzero self-requested a review November 17, 2023 18:47
@lastzero lastzero added please-test Ready for acceptance test and removed waiting Impediment / blocked / waiting labels Nov 17, 2023
@graciousgrey
Copy link
Member

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:

  1. I can't resize the cluster preview reliably, probably because I often don't hit the right position.

  2. Especially on iOS, sometimes the whole page moves up/down when I try to resize the cluster preview.

I've attached a screen recording that shows both behaviors.

resize-handle-recording.mp4

@lastzero @armribdev Any ideas if/how we can improve this?

@graciousgrey graciousgrey added work-in-progress Please don't merge just yet and removed please-test Ready for acceptance test labels Nov 27, 2023
@GlassedSilver
Copy link

@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.

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...?

@lastzero
Copy link
Member

@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?

@GlassedSilver
Copy link

@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!
With a LOT of great things these days that start out humble getting enshittified you really start to get overly anxious, especially when it comes to factors like vendor lock-in.

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. :)

@lastzero
Copy link
Member

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Refactoring, improvement or maintenance task ux Impacts User Experience work-in-progress Please don't merge just yet
Projects
Status: Development 🐝
Development

Successfully merging this pull request may close these issues.

None yet

5 participants