Feature/manga scroll width changer #2980

Marsimplodation · 2024-06-05T16:01:14Z

Added

Added: (Manga Reader) Added the ability to manually override the width the image can scale to. Not applicable to double page layout modes. (Feature Request [Kavita] padding for Mangas (Dynamic image resizing) #2558)

the ability to change the width of manga pages (video 1)
current bugs with this: changing the width scrolls the pages due to the canvas size changing (video 2)

2024-06-05.17-52-04.mp4

2024-06-05.17-52-51.mp4

majora2007

Overall, the idea is pretty good but needs a level of polish and testing of course against all the different renderers and how it plays with things like swipe to paginate.

If you're willing to do the work, I'm willing to merge this in for users.

majora2007 · 2024-06-05T21:09:09Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.html

              </select>
+              <input id="page-fitting-slider" type="range" min=0 max=100 class="form-range visually-hidden" formControlName="widthSlider">


You're going to need to have a label that's invisible for this. You need "" around your values for attr="value".

majora2007 · 2024-06-05T21:09:28Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.ts

@@ -513,6 +513,7 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
        autoCloseMenu: new FormControl(this.autoCloseMenu),
        pageSplitOption: new FormControl(this.pageSplitOption),
        fittingOption: new FormControl(this.mangaReaderService.translateScalingOption(this.scalingOption)),
+        widthSlider: new FormControl(70),


I'd like to have this disabled by default.

majora2007 · 2024-06-05T21:13:35Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.ts

@@ -549,8 +550,31 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
        takeUntilDestroyed(this.destroyRef)
      ).subscribe(() => {});

+      this.generalSettingsForm.get('widthSlider')?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(val => {
+        //update the css width
+        const styleSheets = document.styleSheets;


This isn't needed. You just need the width value from this. So:

this.widthOverride$ = this.generalSettingsForm.get('widthSlider')?.valueChanges.pipe( map(val => val < -1 ? 'initial' : val + '%'), takeUntilDestroyed(this.destroyRef) );

This would give you back the string value. Then on the image you'd do style={{widthOverride$}}.

But the thing is, the image isn't in this component. It is rendered out in the renderers. So while the control is here, it would need to populate the readerSettings and the above code would need to be in each renderer.

You probably also need to disable the input for double page layout.

majora2007 · 2024-06-05T21:14:47Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.ts


-
+      this.generalSettingsForm.get('fittingOption')?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(val => {
+          let slider = document.getElementById("page-fitting-slider") as HTMLInputElement;


You don't need to do this type of thing in Angular. We should enable/disable only. We shouldn't hide it from the UI.

Also keep the subscribe blocks empty. Put this in a tap() so that subscribe can be empty.

majora2007 · 2024-06-05T21:15:32Z

UI/Web/src/app/manga-reader/_models/reader-enums.ts

@@ -1,7 +1,8 @@
 export enum FITTING_OPTION {
    HEIGHT = 'full-height',
    WIDTH = 'full-width',
-    ORIGINAL = 'original'
+    ORIGINAL = 'original',
+    MANUAL = 'manual'


I don't think we need this at all. I think we can just have a slider that applies an arbitrary width with the user is in WIDTH fitting.

Marsimplodation · 2024-06-06T06:00:03Z

Overall, the idea is pretty good but needs a level of polish and testing of course against all the different renderers and how it plays with things like swipe to paginate.

If you're willing to do the work, I'm willing to merge this in for users.

I'd love to the work, I will just need a while to make these changes. I will get back to you when they are done

Marsimplodation · 2024-06-06T18:46:00Z

the last commit changes the feature to only work on the infinite scroll manga-reader for now, while fixing the scrolling issue.
More renderers to come

Tested on mobile and desktop

2024-06-06.12-25-44.mp4

Marsimplodation · 2024-06-06T20:18:42Z

feature now works in all three single page modes (up/down, right/left, infinite scroll) and is disabled/does nothing in everything else, as it has no implementation there
for up/down, right/left the width changing will now work when the height fitting option is used, due to it defeating the point.
I did not experience an issue with swiping to scroll with this

Also are we sure we don't want to hide the slider per default and only show it for the supported modes? If yes the feature works on my part, let me know if there is a change that needs to be done

majora2007 · 2024-06-06T21:04:12Z

UI/Web/src/app/manga-reader/_components/single-renderer/single-renderer.component.ts

+  /**
+   * Width overwrite for maunal width control
+  */
+  widthOverride$ : string = 'none';


The $ sign means that the variable is an observable. The idea is to map it to the widthSlider valueChanges and use map to transform it to the style you need.

majora2007 · 2024-06-06T21:04:30Z

UI/Web/src/app/manga-reader/_components/single-renderer/single-renderer.component.ts

+    this.mangaReaderComponent.generalSettingsForm.get("fittingOption")?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(val => {
+      var img = this.document.getElementById('image-1');
+      if(!img) return;
+      img.style.width = "";


Please always use single quotes in javascript for this project.

majora2007 · 2024-06-06T21:05:53Z

UI/Web/src/app/manga-reader/_components/single-renderer/single-renderer.component.ts

+      else img.style.width = this.widthOverride$;
+    });
+    //reset manual width when fitting option changes
+    this.mangaReaderComponent.generalSettingsForm.get("fittingOption")?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(val => {


This isn't how you should code it. Don't pass the parent component into the child. You can get fittingOption from readerSettings$ and if you can't, update the code so it's packed in the ReaderSetting interface.

so from the mangareader component updating the setting and in the renderers subscribing to the settings value to make changes?

Yes, see imageFitClass$ in the double-renderer and my comment below.

majora2007 · 2024-06-06T21:07:23Z

UI/Web/src/app/manga-reader/_components/single-renderer/single-renderer.component.ts

+      var scaling = this.mangaReaderComponent.generalSettingsForm.get("fittingOption")?.value;
+      if (FITTING_OPTION.HEIGHT == scaling) return;
+
+      if(this.widthOverride$ == 'none') img.style.width = "";


We don't need to set things on the image like this. This should map to a width value (percentage or none) and then that can be set directly on the tag with style="width: {{widthOverride$ | async}}". This will then be "width: none" or "width: 20%"

can you explain this further? I am not experienced in this area and when I try this like you say I get "cannot assign to 'style' because it is an import.

The idea is that in Angular we don't set things on DOM nodes directly, so instead of setting width like this through logic, instead we want to store the width value in an observable (widthOverride$) then in the template, directly use that value like I showed above.

You can see something similar for imageFitClass$.

I see, as someone normally only doing procedural programming in C++ this whole webdev angular stuff is very confusing to me. Hopefully I get the code to your liking the next time
The logic itself should work fine, even deployed it on my own server.

majora2007 · 2024-06-06T21:08:28Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.html

@@ -194,6 +194,8 @@
                <option value="full-width">{{t('width')}}</option>
                <option value="original">{{t('original')}}</option>
              </select>
+              <label for="page-fitting-slider" class="form-label"></label>


Label needs text inside it. I'm open to the label name.

majora2007 · 2024-06-06T21:08:52Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.html

@@ -194,6 +194,8 @@
                <option value="full-width">{{t('width')}}</option>
                <option value="original">{{t('original')}}</option>
              </select>
+              <label for="page-fitting-slider" class="form-label"></label>
+              <input id="page-fitting-slider" type="range" min="-1" max="100" class="form-range" formControlName="widthSlider">


What is the min of -1 for? Wouldn't 0 mean "none"?

majora2007

Still needs a bit of work. Please also test against the double readers. The input needs to be disabled for those modes very likely.

majora2007 · 2024-06-07T11:50:15Z

One thing I also just thought about in addition to double page layout having it disabled, canvas renderer (which is used for SplitOptions.LTR/RTL) needs to disable the field as well.

So to recap, the field should be disabled whenever:
SplitOption is LtR/RtL,
Renderer is any form of Double
Fit is for Height (and Original perhaps)

…not using the slider

Marsimplodation · 2024-06-09T14:30:43Z

Hey, did the rewrite in the style you wanted and this feature is only enabled for, specific combinations:
single renderer & Fit to screen splitting & width option
infinite scroll renderer & fit to screen splitting & width option
otherwise it sets the slider value to 0, which resets the width override and disables the slider

Hope this is more to your liking

majora2007

I'm pulling this down to give it an actual test as well. Code is looking pretty good, just a few minor things.

majora2007 · 2024-06-28T13:09:32Z

UI/Web/src/app/manga-reader/_components/infinite-scroller/infinite-scroller.component.ts

+      this.currentPageElem= this.document.querySelector('img#page-' + this.pageNum);
+      if(!this.currentPageElem) return
+      var images = Array.from(document.querySelectorAll('img[id^="page-"]')) as HTMLImageElement[];
+      for(let i = 0; i < images.length; i++) {


Please use newlines after if (...) branch and before for loops and return statements. Also new line after a closing }. It helps make reading the code much easier.

majora2007 · 2024-06-28T13:10:03Z

UI/Web/src/app/manga-reader/_components/infinite-scroller/infinite-scroller.component.ts

+      this.currentPageElem= this.document.querySelector('img#page-' + this.pageNum);
+      if(!this.currentPageElem) return
+      var images = Array.from(document.querySelectorAll('img[id^="page-"]')) as HTMLImageElement[];
+      for(let i = 0; i < images.length; i++) {


You probably just want to use a foreach loop so we don't need var img = images[i];

majora2007 · 2024-06-28T13:10:20Z

UI/Web/src/app/manga-reader/_components/infinite-scroller/infinite-scroller.component.ts

+      if(!this.currentPageElem) return
+      var images = Array.from(document.querySelectorAll('img[id^="page-"]')) as HTMLImageElement[];
+      for(let i = 0; i < images.length; i++) {
+        var img = images[i];


Don't use var. Always use let and const.

UI/Web/src/app/manga-reader/_components/infinite-scroller/infinite-scroller.component.ts

majora2007 · 2024-06-28T13:11:14Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.html

@@ -194,6 +194,8 @@
                <option value="full-width">{{t('width')}}</option>
                <option value="original">{{t('original')}}</option>
              </select>
+              <label for="page-fitting-slider" class="form-label">Manual Width</label>


You need localization on the label.

majora2007 · 2024-06-28T13:11:40Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.html

@@ -194,6 +194,8 @@
                <option value="full-width">{{t('width')}}</option>
                <option value="original">{{t('original')}}</option>
              </select>
+              <label for="page-fitting-slider" class="form-label">Manual Width</label>
+              <input id="page-fitting-slider" type="range" min="0" max="100" class="form-range" formControlName="widthSlider">


I think we shouldn't allow 0. I think range from 1-100 is appropriate.

refering to the other comment as well, you suggested to let 0 be the value to turn it off, so if 0 is inputed the default behaviour of the width mode is restored. That is also why it defaults to 0 instead of 100%

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.ts

majora2007 · 2024-06-28T15:33:08Z

Some follow up:
Let's move the slider down. Although it's tied to width, it doesn't look good in the current location. This will also give it padding (otherwise you should have added a mt-2)

When I switched from Fit to Hight to Width, the slider value was 0, but the functionality was 100%.

I believe the slider can (but we might need to do with css) but it might be nice to have the 0% be disabled and 100% be default. 0% and 100% are honestly the same though, are they not?

I'm seeing this disabled on the infinite scroller if I came from a mode in which it wasn't fit to width. I think we should have an override if we are in infinite scroller mode to enable it.

I'm seeing the continuous reader (scroll into to change chapter) broken.

I'm seeing on next chapter load, the width is not being set immediately and needs a value change to reapply (this is in webtoon reader).

These are preliminary results from testing. I'll do more in a bit.

majora2007 · 2024-06-28T19:50:08Z

UI/Web/src/assets/langs/en.json

@@ -1695,7 +1695,8 @@
        "image-scaling-label": "Image Scaling",
        "height": "Height",
        "width": "Width",
-        "manual_width": "Manual Width",
+        "width_override": "Width Override",


You need to match the naming scheme of the other localization keys. override-width-label and off should be the keys.

majora2007 · 2024-06-28T19:50:17Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.ts

@@ -568,6 +572,13 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
        this.generalSettingsForm.get('widthSlider')?.setValue(0);
        this.generalSettingsForm.get('widthSlider')?.disable();
      }
+
+            this.widthOverrideLabel$ = this.readerSettings$?.pipe(


Fix indentation

majora2007 · 2024-06-28T19:51:21Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.html

@@ -278,6 +276,13 @@
                     min="10" max="100" step="1" formControlName="darkness">
            </div>

+            <div class="col-md-6 col-sm-12">
+              <label for="page-fitting-slider" class="form-label">{{t('width_override')}}:
+                  {{ (widthOverrideLabel$ | async) ? (widthOverrideLabel$ | async) : t('width_override_off') }}


We can probably re-write this completely as:
@if (widthOverrideLabel$ | async; as widthOverrideLabel) {
{{ widthOverrideLabel ? widthOverrideLabel : t('off') }}
}

majora2007 · 2024-06-28T19:51:51Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.html

+              <label for="page-fitting-slider" class="form-label">{{t('width_override')}}:
+                  {{ (widthOverrideLabel$ | async) ? (widthOverrideLabel$ | async) : t('width_override_off') }}
+              </label>
+              <input id="page-fitting-slider" type="range" min="0" max="100" class="form-range" formControlName="widthSlider">


let's rename these ids to be width-override-slider instead

majora2007 · 2024-06-28T19:57:32Z

After your last push, I see a few more issues:
Infinite Scroller:

Continuous reader (next) doesn't trigger anymore
You can now change the Fitting which previously didn't do anything, now can break the webtoon reader

majora2007

A few more comments then I'm good to merge

majora2007 · 2024-06-28T21:02:32Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.ts

@@ -549,9 +554,33 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
        takeUntilDestroyed(this.destroyRef)
      ).subscribe(() => {});

+      this.generalSettingsForm.get('pageSplitOption')?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(val => {
+        const fitting = this.generalSettingsForm.get('fittingOption')?.value;
+        if(PageSplitOption.FitSplit == val && FITTING_OPTION.WIDTH == fitting) {


space after if. If ( and also newline above if statements please

majora2007 · 2024-06-28T21:03:40Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.ts

@@ -549,9 +554,33 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
        takeUntilDestroyed(this.destroyRef)
      ).subscribe(() => {});

+      this.generalSettingsForm.get('pageSplitOption')?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(val => {


Let's move the subscribe into a tap so that subscribe can be empty.

In addition, move each pipe into a new line, like you see elsewhere.

I would also get the control in a variable then re-use in the code, so:
const widthOverrideControl = this.generalSettingsForm.get('widthSlider')!;

majora2007 · 2024-06-28T21:04:18Z

UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.ts

@@ -549,9 +554,33 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
        takeUntilDestroyed(this.destroyRef)
      ).subscribe(() => {});

+      this.generalSettingsForm.get('pageSplitOption')?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(val => {
+        const fitting = this.generalSettingsForm.get('fittingOption')?.value;
+        if(PageSplitOption.FitSplit == val && FITTING_OPTION.WIDTH == fitting) {


Let's add a bit of documentation about our decisions about when this should work. It will likely help me in the future when there is a bug.

majora2007 · 2024-06-28T21:04:44Z

UI/Web/src/app/manga-reader/_components/single-renderer/single-renderer.component.ts

@@ -53,6 +53,11 @@ export class SingleRendererComponent implements OnInit, ImageRenderer {
  pageNum: number = 0;
  maxPages: number = 1;

+  /**
+   * Width overwrite for maunal width control


Width override*

Marsimplodation · 2024-06-28T21:41:40Z

did these changes

majora2007

Looks good. I'll merge this in and get feedback from nightly users. Appreciate the work.

Marsimplodation added 3 commits June 5, 2024 17:50

added the abbility to change size

99d3dbf

forgot translation in last commit

7dab0b3

removed accidental import

9da64bf

Marsimplodation mentioned this pull request Jun 5, 2024

"Volume 100000 Chapter -100000" Label #2981

Closed

majora2007 requested changes Jun 5, 2024

View reviewed changes

just the manga reader for now

df83627

works on all single page renderers in the manga reader

802e865

Marsimplodation requested review from majora2007 June 6, 2024 20:19

majora2007 reviewed Jun 6, 2024

View reviewed changes

majora2007 requested changes Jun 6, 2024

View reviewed changes

majora2007 added this to In progress in v0.8 - PDF & Comic Love via automation Jun 7, 2024

majora2007 moved this from In progress to To do in v0.8 - PDF & Comic Love Jun 7, 2024

Marsimplodation added 3 commits June 7, 2024 19:33

moved to a more angular style [WIP]

183f3df

disabled input for non relevant readers and fixed flickering

b18ebd5

feature should now be working, fixed UI bug and reset the style when …

16596d2

…not using the slider

Marsimplodation requested review from majora2007 June 9, 2024 14:30

fixed a typo

49714d1

majora2007 requested changes Jun 28, 2024

View reviewed changes

Marsimplodation added 2 commits June 28, 2024 16:42

localication and minor codestyle

de97aae

minor codestyle again

8f5ed1b

Marsimplodation added 2 commits June 28, 2024 19:12

requested bug fixes

e127b78

revert unnecessary change and style fix

1fbdecc

majora2007 reviewed Jun 28, 2024

View reviewed changes

Marsimplodation added 2 commits June 28, 2024 22:01

requested changes in naming

5f80d0c

fixed splitting

3a3a7dc

majora2007 requested changes Jun 28, 2024

View reviewed changes

Marsimplodation added 2 commits June 28, 2024 23:38

typo, documentation and code style

dca985d

removed unecessary new line

03e3295

Marsimplodation requested a review from majora2007 June 28, 2024 21:41

majora2007 approved these changes Jun 28, 2024

View reviewed changes

majora2007 changed the base branch from develop to feature/reader-changes June 29, 2024 15:28

majora2007 merged commit e238403 into Kareadita:feature/reader-changes Jun 29, 2024

v0.8 - PDF & Comic Love automation moved this from To do to Done Jun 29, 2024

		</select>
		<input id="page-fitting-slider" type="range" min=0 max=100 class="form-range visually-hidden" formControlName="widthSlider">



		this.generalSettingsForm.get('fittingOption')?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(val => {
		let slider = document.getElementById("page-fitting-slider") as HTMLInputElement;

Feature/manga scroll width changer #2980

Feature/manga scroll width changer #2980

Conversation

Marsimplodation commented Jun 5, 2024 • edited by majora2007 Loading

Added

majora2007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marsimplodation commented Jun 6, 2024

Marsimplodation commented Jun 6, 2024 • edited Loading

Marsimplodation commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majora2007 left a comment

Choose a reason for hiding this comment

majora2007 commented Jun 7, 2024

Marsimplodation commented Jun 9, 2024

majora2007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marsimplodation Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

majora2007 commented Jun 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majora2007 commented Jun 28, 2024

majora2007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marsimplodation commented Jun 28, 2024

majora2007 left a comment

Choose a reason for hiding this comment

Marsimplodation commented Jun 5, 2024 •

edited by majora2007

Loading

Marsimplodation commented Jun 6, 2024 •

edited

Loading

Marsimplodation Jun 28, 2024 •

edited

Loading