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

Fix reader width #567

Merged
merged 8 commits into from
Jan 26, 2024
Merged

Conversation

chancez
Copy link
Contributor

@chancez chancez commented Jan 24, 2024

The margin: auto on the Box container for the image was preventing 100% width to actually mean 100%.

Now that 100% is actually possible, I think fitPageToWindow makes more sense as a default. Additionally, since the image can fill 100% of the page, it can cover the ReaderNavBar, so set the z-index of the ReaderNavBar so it's rendered on top of the image and clickable.

Before.

Notice 100%, is not actually taking 100% of the page width.

Screenshot 2024-01-23 at 7 07 17 PM

After:

fit to page width
Screenshot 2024-01-23 at 7 04 21 PM

21% width
Screenshot 2024-01-23 at 7 04 14 PM

100% width
Screenshot 2024-01-23 at 7 04 06 PM

@schroda
Copy link
Collaborator

schroda commented Jan 24, 2024

the single page does not take 100% of the width and is not centered when the height is < 100vh

@chancez
Copy link
Contributor Author

chancez commented Jan 24, 2024

the single page does not take 100% of the width and is not centered when the height is < 100vh

Ah your right, I gotta update the other pagers.

The margin: auto on the Box container for the image was preventing 100% width
to actually mean 100%.

Now that 100% is actually possible, I think fitPageToWindow makes more
sense as a default. Additionally, since the image can fill 100% of the
page, it can cover the ReaderNavBar, so set the z-index of the
ReaderNavBar so it's rendered on top of the image and clickable.

Signed-off-by: Chance Zibolski <[email protected]>
@chancez chancez force-pushed the pr/chancez/reader_width_fill branch from 60689cd to 74f9032 Compare January 24, 2024 19:15
@chancez
Copy link
Contributor Author

chancez commented Jan 24, 2024

So I fixed it for the single page layout. I was gonna update the DoublePage layouts to support adjustable width, but I cannot figure out how to make it take 100% of the width with horizontal layout because the flex direction has to be row.

@chancez
Copy link
Contributor Author

chancez commented Jan 24, 2024

Ah I think I figured that out, it's related to the height set in the imageStyle in DoublePage.tsx

@chancez
Copy link
Contributor Author

chancez commented Jan 24, 2024

Ok got double paged readers to work with the reader width setting too.

@chancez chancez force-pushed the pr/chancez/reader_width_fill branch from 8933bff to e0e914b Compare January 24, 2024 20:58
src/components/reader/DoublePage.tsx Outdated Show resolved Hide resolved
src/components/reader/Page.tsx Show resolved Hide resolved
@chancez chancez force-pushed the pr/chancez/reader_width_fill branch 4 times, most recently from 55c5cdd to e795654 Compare January 25, 2024 21:45
@schroda
Copy link
Collaborator

schroda commented Jan 25, 2024

now the pages of the double pager do not take up 100% of the width anymore 😅

@chancez chancez force-pushed the pr/chancez/reader_width_fill branch from e795654 to 4514c91 Compare January 26, 2024 01:55
@chancez
Copy link
Contributor Author

chancez commented Jan 26, 2024

CSS is hard. flex-grow: 1 fixes that. I'm not sure I fully understand why though, but I verified it at least maintains the aspect ratio of the images correctly.

In case the parent container is flex row, the container does not automatically take up 100% of the available width, thus, the page also was not able to take up 100% of the width
The set reader width can't be applied to each page of the double pages because otherwise it will already take up 100% of the available width with the setting only being at 50%.

Instead, the set width has to be divided by 2, so that both pages take up the set reader width
@schroda schroda merged commit 4d75474 into Suwayomi:master Jan 26, 2024
1 check passed
@chancez chancez deleted the pr/chancez/reader_width_fill branch January 26, 2024 22:17
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