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

Double page laod speedup & Fullscreen & Mouse wheel move #599

Merged
merged 17 commits into from
Apr 9, 2022
Merged

Double page laod speedup & Fullscreen & Mouse wheel move #599

merged 17 commits into from
Apr 9, 2022

Conversation

djacks6278
Copy link
Contributor

canvas draw is so slow and heavy when load high resolution image...
so change this code like single page view

no more need to draw in canvas

and i change imagemap to jquery
outer-left, outer-right is now work perfectly

we don't need canvas nomore...
canvas draw is so slow and heavy
we don't need canvas nomore...
canvas draw is so slow and heavy
now when popup is open page don't change.
and some position issue fix
i add some fullscreen on pc web and new event key press F(fullscreen toggle)
and when u active fullscreen u can use a mouse wheel to move. enjoy :)
@djacks6278 djacks6278 changed the title Double page view laod speedup Double page laod speedup & Fullscreen & Mouse wheel move Apr 4, 2022
Copy link
Owner

@Difegue Difegue left a comment

Choose a reason for hiding this comment

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

👋 Thanks a lot for willing to contribute with Reader improvements! 🙏
As you've probably seen it's a bit complex, so I'll gladly accept the work.

Most of the changes here are good (I especially appreciate fullscreen mode!), but I tested the PR and found a few issues that have to be fixed before I can merge this.

templates/reader.html.tt2 Outdated Show resolved Hide resolved
templates/reader.html.tt2 Outdated Show resolved Hide resolved
public/themes/modern.css Outdated Show resolved Hide resolved
public/themes/modern.css Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
@djacks6278
Copy link
Contributor Author

@Difegue i don't know how close the your review
but i resolve all thing in your review

so u need check again!

@djacks6278 djacks6278 requested a review from Difegue April 5, 2022 17:16
Copy link
Owner

@Difegue Difegue left a comment

Choose a reason for hiding this comment

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

Lookin' good and almost bug free it seems! 👀
A few more nitpicks from me and I think we can merge this afterwards.

public/themes/modern.css Outdated Show resolved Hide resolved
public/css/lrr.css Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Show resolved Hide resolved
public/js/reader.js Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
public/js/reader.js Outdated Show resolved Hide resolved
@djacks6278
Copy link
Contributor Author

i fixed all review well i'm too late sorry about that

@Difegue
Copy link
Owner

Difegue commented Apr 9, 2022

Yup, looks great! 🎊
Once again, thanks a ton for taking the time to work on this.

@Difegue Difegue merged commit 7e05bbd into Difegue:dev Apr 9, 2022
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.

2 participants