Skip to content

fix: improve image preview functionality and hover behavior #39

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: improve image preview functionality and hover behavior #39

wants to merge 1 commit into from

Conversation

iAmNsengi
Copy link

Description :

  • Fix inconsistent hover behavior in detail view
  • Add proper positioning calculation
  • Improve transition animations
  • Fix edge case issues with window boundaries
  • Replace "ICON..." text with visual image group icon
  • Add grid layout for multiple images preview
  • Improve hover states and transitions
  • Better visual feedback for image interactions

closes #35

@soif
Copy link
Collaborator

soif commented May 15, 2025 •
edited
Loading

Hey thanks for the PR, you were fast ! 😎 👍

I've not tested it yet, but would please see my observations.
Also can you repost the CSS, with ONLY your changes (whithout all indentations changes) to ease the diff display on GH. Then, once merged. Feel free to add another PR to just fix the indentations.

IGIRANEZAJosue
IGIRANEZAJosue reviewed May 15, 2025
View reviewed changes
static/js/toh_main.js Outdated
Comment on lines 986 to 994
const offset = $link.offset();
const linkWidth = $link.outerWidth();
const linkHeight = $link.outerHeight();
const containerWidth = $container.outerWidth();
const containerHeight = $container.outerHeight();
const windowWidth = $(window).width();
const windowHeight = $(window).height();
const scrollTop = $(window).scrollTop();

Copy link

Choose a reason for hiding this comment

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

I believe these changes, including the indentation adjustments, may not be necessary in the context of the issue.

@iAmNsengi
Copy link
Author

Hey thanks for the PR, you were fast ! 😎 👍

I've not tested it yet, but would please see my observations. Also can you repost the CSS, with ONLY your changes (whithout all indentations changes) to ease the diff display on GH. Then, once merged. Feel free to add another PR to just fix the indentations.

Hello @soif , the requested changes were fixed you might proceed with reviewing the PR.

Thanks

@soif soif mentioned this pull request Jun 15, 2025
Closed
@soif
Copy link
Collaborator

soif commented Jun 15, 2025

@iAmNsengi Thank you for the PR.

Unfortunately, OpenWrt team has recently changed something on their site that now prevents remote image loading. So i can not review this PR at this time, until they will fix it (see #41).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IGIRANEZAJosue IGIRANEZAJosue IGIRANEZAJosue left review comments

@albert-byiringiro albert-byiringiro albert-byiringiro approved these changes

@Angekarara Angekarara Angekarara approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@iAmNsengi @soif @albert-byiringiro @IGIRANEZAJosue @Angekarara