Skip to content

Eemeli/enhancement/616 custom comic reader modal#635

Open
EemeliJ wants to merge 4 commits intodevfrom
Eemeli/enhancement/616-custom-comic-reader-modal
Open

Eemeli/enhancement/616 custom comic reader modal#635
EemeliJ wants to merge 4 commits intodevfrom
Eemeli/enhancement/616-custom-comic-reader-modal

Conversation

@EemeliJ
Copy link
Copy Markdown
Contributor

@EemeliJ EemeliJ commented Mar 23, 2026

📄 Pull Request Overview

Closes #616

🔧 Changes Made

  1. Replaced Fancybox with a custom ReaderModal and implemented a comic reader with navigation, slider, responsive layouts (desktop 2-page / mobile 1-page), and touch swipe support.

  2. Removed legacy Fancybox-related logic and simplified modal handling to use React state and the HTML5 element.


Checklist Before Submission

  • Functionality: I have tested my code, and it works as expected. ✅
  • JSDoc: I have added or updated JSDoc comments for all relevant code. ✅
  • Debugging: No console.log() or other debugging statements are left. ✅
  • Clean Code: Removed commented-out or unnecessary code. ✅
  • Tests: Added new tests or updated existing ones for the changes made. ✅
  • Documentation: Documentation has been updated (if applicable). ✅

📝 Additional Information

Provide any additional context or information that reviewers may need to know:

The current gallery component serves as a working reference implementation, while the ReaderModal is fully reusable and intended for use in the upcoming redesigned page.

  • Screenshots: [Include any screenshots or videos if the changes affect the UI]
previewpage modalpage1 modalpage2 modalfinalpage modalmobile
  • Dependencies:
    No new dependencies introduced; Fancybox removed.

  • Known Issues:
    The current gallery component is a feature-specific implementation and serves as a reference for the upcoming page refactor.

EemeliJ added 3 commits March 13, 2026 13:20
…ntation, made changes to the GalleryCategoriesWithModalSlider component to manage modal state with React.
…d desktop 2-page and mobile 1-page layouts, numbering now works correctly on mobile, image sizing improved in modal, touch swipe support for mobile added.
@EemeliJ EemeliJ changed the base branch from main to dev March 24, 2026 10:35
Copy link
Copy Markdown
Contributor

@Rutjake Rutjake left a comment

Choose a reason for hiding this comment

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

Great work! Apologies for the delay in reviewing this. The comic should enlarge when clicked to ensure a smooth user experience. To wrap up the page updates, it would be great if you could also use the correct icons and ensure they are in the right positions.

# local env files
.env*.local
.env
.env.*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No changes needed to .gitignore, as it already covers our environment files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There were some issues with pushing at one point, caused by git trying to push secrets for some unknown reason despite .gitignore, I’ll remove it going forward 👍

…licking on the comic, and closed by clicking on the backdrop.
@EemeliJ EemeliJ requested a review from Rutjake April 2, 2026 12:12
Copy link
Copy Markdown
Member

@Skoivumaki Skoivumaki left a comment

Choose a reason for hiding this comment

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

Great work! Did not find any major issues, but could add fix for following:
Since the enlarged modals size is determined by window width, in odd aspect-ratio cases the modal will remain very small and hard to read. (see example. 4k monitor)

Image

@Rutjake Modal works great, but now we can see the pages changing on the background. We could enhance this more, since the initial "pages on the page" loses its meaning with this modal thing?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants