Skip to content

UX: Add preview image to the labels tab in the photo edit dialog #3532

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

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

reubendowle
Copy link
Contributor

@reubendowle reubendowle commented Jul 9, 2023

This pull requests adds an image preview when editing labels.

image

This is to address this issue: #628

The issue points out that there is limited space for thumbnails on mobile devices, so it will be hidden on mobile devices.

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 Jul 9, 2023

CLA assistant check
All committers have signed the CLA.

@lastzero
Copy link
Member

lastzero commented Jul 9, 2023

Thanks! We'll provide more feedback when we are back in our office.

@reubendowle reubendowle force-pushed the add-thumbnail-to-labels branch from a03cf90 to b98ce5d Compare July 9, 2023 22:29
@lastzero
Copy link
Member

@reubendowle I'll merge this, but need to refactor it as $isMobile only indicates if it's a mobile browser e.g. with touch event support, so doesn't depend on the size of the browser window, which is the actual concern:

const isMobile =
/Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent) ||
(navigator.maxTouchPoints && navigator.maxTouchPoints > 2);

@lastzero lastzero added enhancement Enhancement or improvement of an existing feature ux Impacts User Experience labels Jul 20, 2023
@lastzero lastzero changed the title Frontend: Add image preview when editing labels Frontend: Show thumbnail in the Labels tab of the edit dialog Jul 20, 2023
@lastzero lastzero merged commit 3d828a7 into photoprism:develop Jul 20, 2023
@graciousgrey
Copy link
Member

Ideally we can also add a thumbnail to the ⚙️ tab.

@lastzero
Copy link
Member

@graciousgrey I'll check if that's possible. It might be good to add a component for this so we don't have to copy and paste the code into each tab....

@lastzero lastzero changed the title Frontend: Show thumbnail in the Labels tab of the edit dialog UX: Show preview image in the labels tab of the photo editing dialog Jul 20, 2023
@lastzero lastzero changed the title UX: Show preview image in the labels tab of the photo editing dialog UX: Show preview image in the labels tab of the edit dialog Jul 20, 2023
lastzero added a commit that referenced this pull request Jul 20, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero lastzero added the please-test Ready for acceptance test label Jul 20, 2023
@lastzero lastzero changed the title UX: Show preview image in the labels tab of the edit dialog UX: Add preview image to the labels tab in the photo edit dialog Jul 20, 2023
@lastzero
Copy link
Member

Updated our release notes and the preview build, so you can test this:

lastzero added a commit that referenced this pull request Jul 20, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@reubendowle
Copy link
Contributor Author

@reubendowle I'll merge this, but need to refactor it as $isMobile only indicates if it's a mobile browser e.g. with touch event support, so doesn't depend on the size of the browser window, which is the actual concern:

@lastzero personally this looks fine for me at most sizes of the browser window on PC. I guess when the width gets too narrow triggers a scrollbar:

image

image

But this is the same behavior as the details tab. So maybe it can just be left like that, and maybe remove the isMobile condition completely?

@lastzero
Copy link
Member

I've already merged and refactored it yesterday. It's now deployed on our demo and available with our preview build.

@loulou91
Copy link

Useful feature, but (for me) the most useful would be to have it on "people" tab...

@lastzero
Copy link
Member

@loulou91 Instead of rendering the image on each tab and thus reducing the space available for the actual content, I was thinking about a statically positioned preview e.g. in the lower left corner. However, we just merged a lot of PR and are still busy making them releasable, so this will have to wait a bit.

@graciousgrey graciousgrey added tested Changes have been tested successfully released Available in the stable release and removed please-test Ready for acceptance test labels Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement of an existing feature released Available in the stable release tested Changes have been tested successfully ux Impacts User Experience
Projects
Status: Release 🌈
Development

Successfully merging this pull request may close these issues.

5 participants