-
-
Notifications
You must be signed in to change notification settings - Fork 341
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
Bugfix: Scrolling functionality on short webtoons #3006
base: develop
Are you sure you want to change the base?
Bugfix: Scrolling functionality on short webtoons #3006
Conversation
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.
Annotations.
} | ||
|
||
if (event.deltaY > 0) { | ||
this.loadNextChapter.emit(); |
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.
I didn't update the value of this.scrollingDirection
here -- not sure if this is needed. :)
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.
Yeah, I think it is a good idea so you know the direction you were last scrolling. It wont matter much, but better to have.
@Zackareee - I just wanted to reach out and let you know that we've seen the PR's you submitted and aren't ignoring them. The main dev is on vacation right now and will be able to review these as soon as he gets back. Didn't want you to feel discouraged because you haven't gotten a response yet. |
Would you be able to DM me a file to test against? |
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.
@@ -13,7 +13,7 @@ | |||
<strong>Scroll Top:</strong> {{getScrollTop()}} | |||
</div> | |||
|
|||
<div *ngIf="atTop" #topSpacer class="spacer top" role="alert" (click)="loadPrevChapter.emit()"> | |||
<div *ngIf="atTop || heightLessThanView()" #topSpacer [style.height]="heightLessThanView() ? 'inherit' : '300px'" class="spacer top" role="alert" (click)="loadPrevChapter.emit()"> |
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.
Help me understand the arbitrary 300px? Is this just from the styles? If so, let's make it a css variable and use var(--) on the property so I don't forget to update it in multiple places.
} | ||
|
||
if (event.deltaY > 0) { | ||
this.loadNextChapter.emit(); |
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.
Yeah, I think it is a good idea so you know the direction you were last scrolling. It wont matter much, but better to have.
I forgot about this PR, I will note to come back and re-review. |
Fixed
When viewing a short webtoon, the previous chapter button is not displayed. Also, scrolling does not go to the next/previous chapter as expected.
To fix, if a webtoon is smaller than the viewport, force the previous chapter button to display. Also, use a separate listener to handle the scroll event.