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

Update reader width live when updating in reader view #581

Merged

Conversation

chancez
Copy link
Contributor

@chancez chancez commented Jan 27, 2024

Basically, instead of updating the width of the image after clicking ok, the width is adjusted live, which makes it easier to figure out the right value for the manga being viewed.

Ideally we could revert the changes with the cancel button, but to do that requires tracking the value prior to the updates. With how the reader is designed currently, everything is using settings directly and so it's tricky to separate the settings being used to draw the component and the settings that are saved. So instead I've disabled the cancel button when liveUpdate is true, since settings are updated immediately.

While implementing this, I realized we can't update the backend every time we re-render when liveUpdate is true, so I've updated the logic for persisting settings to support skipping the API call to save the settings in the backend, and only perform the API it when the user clicks ok on the number setting.

@chancez chancez force-pushed the pr/chancez/live_update_width_reader_settings branch 4 times, most recently from edc0b3d to d3b9a0e Compare January 27, 2024 05:26
src/components/navbar/ReaderNavBar.tsx Outdated Show resolved Hide resolved
src/components/reader/ReaderSettingsOptions.tsx Outdated Show resolved Hide resolved
src/components/settings/NumberSetting.tsx Outdated Show resolved Hide resolved
src/components/settings/NumberSetting.tsx Outdated Show resolved Hide resolved
src/components/navbar/ReaderNavBar.tsx Outdated Show resolved Hide resolved
src/components/reader/ReaderSettingsOptions.tsx Outdated Show resolved Hide resolved
src/components/settings/NumberSetting.tsx Show resolved Hide resolved
src/screens/Reader.tsx Outdated Show resolved Hide resolved
src/components/settings/NumberSetting.tsx Outdated Show resolved Hide resolved
src/components/settings/NumberSetting.tsx Outdated Show resolved Hide resolved
@schroda
Copy link
Collaborator

schroda commented Jan 27, 2024

not sure if that is fixable with the way the reader and reader settings are implemented, but it does feel kinda laggy (if you look at the slider compared to the mouse position)

@chancez chancez force-pushed the pr/chancez/live_update_width_reader_settings branch 2 times, most recently from 1df2eb8 to 70b879c Compare January 27, 2024 18:07
@chancez
Copy link
Contributor Author

chancez commented Jan 27, 2024

@schroda Okay I took another stab, added cancel back and switched handleUpdate to the previous behavior and added a new optional handleLiveUpdate callback prop to update the setting without persisting.

@chancez chancez force-pushed the pr/chancez/live_update_width_reader_settings branch from 70b879c to 841d2b9 Compare January 27, 2024 18:08
src/components/settings/NumberSetting.tsx Show resolved Hide resolved
src/components/settings/NumberSetting.tsx Show resolved Hide resolved
src/screens/Reader.tsx Outdated Show resolved Hide resolved
src/screens/settings/DefaultReaderSettings.tsx Outdated Show resolved Hide resolved
src/components/settings/NumberSetting.tsx Outdated Show resolved Hide resolved
@chancez chancez force-pushed the pr/chancez/live_update_width_reader_settings branch from 841d2b9 to 1e1b287 Compare March 17, 2024 23:17
@chancez
Copy link
Contributor Author

chancez commented Mar 17, 2024

Pushed after rebasing and fixing what I could so I can get a re-review, but need to test things again.

@chancez chancez force-pushed the pr/chancez/live_update_width_reader_settings branch from 1e1b287 to 6be1e0e Compare March 17, 2024 23:24
@chancez
Copy link
Contributor Author

chancez commented Mar 17, 2024

Ok should be good, I tested and it seems to be working as expected.

src/screens/Reader.tsx Outdated Show resolved Hide resolved
@chancez chancez force-pushed the pr/chancez/live_update_width_reader_settings branch from 6be1e0e to 42305ff Compare March 18, 2024 00:57
Basically, instead of updating the width of the image after clicking
`ok`, the width is adjusted live, which makes it easier to figure out
the right value for the manga being viewed.

While implementing this, I realized we can't update the backend every
time we re-render when `liveUpdate` is true, so I've updated the logic
for persisting settings to support skipping the API call to save the
settings in the backend, and only perform the API it when the user
clicks ok on the number setting.

Signed-off-by: Chance Zibolski <[email protected]>
@chancez chancez force-pushed the pr/chancez/live_update_width_reader_settings branch from 42305ff to 27b8e91 Compare March 18, 2024 16:14
@chancez chancez requested a review from schroda March 18, 2024 16:26
@schroda schroda merged commit 0bf88d3 into Suwayomi:master Mar 18, 2024
1 check passed
@chancez chancez deleted the pr/chancez/live_update_width_reader_settings branch March 19, 2024 18:04
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