getmango / Mango

Mango is a self-hosted manga server and web reader
https://getmango.app
MIT License
1.68k stars 118 forks source link

Fit image options #317

Closed Hiers closed 2 years ago

Hiers commented 2 years ago

Adding the image fit feature mentioned in #147.

The setFit function is hardcoded to look for the css file and attribute on those exact spots and looks likely to break if that file is changed. Asides from this, the only other way I know is to make a function to look for it given the name of the file and attribute, but it'd be a function that would only be used on this one occasion.

And if anyone has a better idea for how the option should appear in the settings I'd like to hear it, design is not my passion.

Hiers commented 2 years ago

Note: the real size option has the added benefit of having the browser's native scaling work as expected.

Leeingnyo commented 2 years ago

Thank you for PR!

The setFit function is hardcoded to look for the css file and attribute on those exact spots and looks likely to break if that file is changed. Asides from this, the only other way I know is to make a function to look for it given the name of the file and attribute, but it'd be a function that would only be used on this one occasion.

We would write styles to classes (such as .vertical, .horizontal, .original), and in JS, adding classes to tags or removing classes from tags according to option value. I saw just codes and didn't see actual implementation on a browser yet. I'll tell you what would be a detailed way (which tags to have classes, what contents that class style would have) after seeing this on a browser.

hkalexling commented 2 years ago

Thanks! Yeah agreed with @Leeingnyo I think accessing the styles directly is a bit too hacky. I took a quick look and I think in the setFit function we can simply do this.fitType = fitType without touching the styles? The img style will be updated in src/views/reader.html.ecr.

Hiers commented 2 years ago

this.setFit is already set in the fitChanged() function and this works to update the values in src/views/reader.html.ecr.

However, mango.css has a max-width attribute to "constrain the element to its parent width", which is something we don't want if the image is to stay in its original size. This is in a separate file and I don't know of a non-hacky way to access it, which is why 80% of my PR comment is just asking for a better idea.

@Leeingnyo had the idea of using styles with tags, but we need to look into it before changing the PR.

Leeingnyo commented 2 years ago

@Hiers Sorry to late!

I misunderstood something. please forget about what i said before.

It seemed you used the hardcoded style to remove this following rule

canvas,
img,
video {
  /* 1 */
  max-width: 100%;
  /* 2 */
  height: auto;
  /* 3 */
  box-sizing: border-box;
}

Rather than using document.styleSheets[0].rules[21].style.maxWidth directly to remove a rule, we could override a css rule like this

<uk-img
  style="
    max-width:${fitType === 'horz' ? '100%' : fitType === 'real' ? 'none' : ''};
    max-height:${fitType === 'vert' ? '100%' : fitType === 'real' ? 'none' : ''};
  "
/>

an inline style rule would override the rule of mango.css and we should apply none to cancel max-width and max-height

Leeingnyo commented 2 years ago

As shown in this picture, the position of a right flip panel isn't on expected position in real mode (when viewer horizontally scrolled)

스크린샷 2022-07-12 오전 10 13 42

it would be fixed giving width: fit-content; to the wrapper div (parent of uk-img).

Edit: 07-12 11:17.

It would be fixed perfectly giving

Hiers commented 2 years ago

Ah, I didn't realise my problem was deleting max-width instead of setting it to none, thank you.

Your edit made me realise I never explicitly said it, but I was making the fit image feature to be for paged mode only, which is what made sense to me. Do you think this should be available in continuous mode too? I thought you were asking me to change something different, oops.

hkalexling commented 2 years ago

Thanks both! I noticed another small issue - the page flip panels are still not positioned correctly. See screenshot below.

DeepinScreenshot_select-area_20220716113915

The right panel should be the right most 1/3 area of the entire screen, and not the right 1/3 of the page. I believe this is caused by the new position: relative style in the parent of uk-img. It looks like it can be fixed by moving position: relative to the uk-section tag instead, since it always has 100% width. Do you mind giving it a try? But let me know if I am missing anything!

Hiers commented 2 years ago

Nice catch! And your fix did work.

hkalexling commented 2 years ago

LGTM, thanks!