-
Notifications
You must be signed in to change notification settings - Fork 64
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
Conversation
…configuration of displaying media view instead of preview in preview tooltip
There was a problem hiding this 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):
3. Overlay + Media View Tooltip:
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.
.../src/main/webapp/WEB-INF/templates/includes/metadataEditor/partials/media-list-overlay.xhtml
Show resolved
Hide resolved
.../src/main/webapp/WEB-INF/templates/includes/metadataEditor/partials/media-list-overlay.xhtml
Outdated
Show resolved
Hide resolved
…or/partials/media-list-overlay.xhtml Co-authored-by: Arved Solth <solth@effective-webwork.de>
…or/partials/media-list-overlay.xhtml Co-authored-by: Arved Solth <solth@effective-webwork.de>
…ultiple checkboxes
@solth Thank you for the feedback. As suggested, the hover behavior in the preview will now be configured through a selection field. |
There was a problem hiding this 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!
Kitodo-DataManagement/src/main/java/org/kitodo/data/database/enums/PreviewHoverMode.java
Show resolved
Hide resolved
.../src/main/webapp/WEB-INF/templates/includes/metadataEditor/partials/media-list-overlay.xhtml
Outdated
Show resolved
Hide resolved
…or/partials/media-list-overlay.xhtml Co-authored-by: Arved Solth <solth@effective-webwork.de>
Without tooltip
With tooltip