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

Display tooltip instead of the overlay in preview #5845

Merged
merged 12 commits into from
Jan 15, 2024

Conversation

markusweigelt
Copy link
Collaborator

@markusweigelt markusweigelt commented Nov 30, 2023

  • The tooltip can be enabled through the project settings. By default, the overlay will still be displayed.

image

  • The tooltip is only displayed when an image type has been selected for the preview view.
  • It´s configurable that the image of media view is rendered in the tooltip (Hence, a preloading mechanism has been implemented to load the media view image only when the tooltip is about to be displayed)
  • The size of the tooltip depends on its content. Images are restricted to a maximum size of 40% of the viewport height.
  • Improve userinterface of folder use to add config for tooltip in project settings.

Without tooltip

image

image

With tooltip

image

image

@solth solth added the feature label Dec 1, 2023
Copy link
Member

@solth solth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works as advertised!

I do have one remark/suggestion about the configuration, though. If I am not mistaken you end up with 3 possible options that can be configured - "overlay", "overlay + preview tooltip", "overlay + media view tooltip".

You use two switches to control these three options of the thumbnail behaviour on mouse hover, though:

1. Just overlay (old behaviour):
Bildschirmfoto 2024-01-08 um 12 46 00

2. Overlay + Preview Tooltip:
Bildschirmfoto 2024-01-08 um 12 46 05

3. Overlay + Media View Tooltip:
Bildschirmfoto 2024-01-08 um 12 45 54

4. ???:
Bildschirmfoto 2024-01-08 um 12 03 31

So to me it seems this fourth switch setting either represents an invalid configuration or one that is already covered by a different state of the switches (probably the same as option 3, "Overlay + Media View Tooltip").

Therefore I think it would be better to have just one configuration element - a pulldown menu with three options or a set of radio buttons, for example - instead of the two separate switches to avoid confusion.

Alternatively, you could keep the switches but deactivate the second switch until the first is set to "true". This seems to be more of a hassle, though, since you would also have to potentially update the value of the second switch (to "false") again when the first is set to "false" again.

@markusweigelt
Copy link
Collaborator Author

@solth Thank you for the feedback. As suggested, the hover behavior in the preview will now be configured through a selection field.

Copy link
Member

@solth solth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good now! Just two more very tiny suggestions, if you don't mind!

markusweigelt and others added 2 commits January 15, 2024 09:08
@solth solth merged commit a6d0a8b into kitodo:master Jan 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants