rNeomy / reader-view

Access Firefox's built in reader view from right click context menu
https://webextension.org/listing/chrome-reader-view.html
Mozilla Public License 2.0
471 stars 74 forks source link

NextChap formatting #179

Closed Shrulik closed 1 year ago

Shrulik commented 1 year ago

Continuing #178

a. I fixed the indentation and added a link to my Github fork, best place to contact regarding this matter.
I will probably keep improving the detection and might extract it into an NPM library at some point. Will make additional pull requests when something interesting happens.

b. The new chapters plugin is more in tune with the rest of the code base, but I would set opening links in the reader as on by default, or at least place the two options together so it is clear to the user. It is somewhat unexpected when you click a next chapter link from inside a reader and this closes the reader as the user was clearly intending to keep on reading.

Another option, is that the first time the user clicks on any of the chapter links for the website, you can ask the user if he wants to add the website to the websites on which the reader opens automatically. Then I'm assuming it will flow using current behavior.

c. Consider adopting two other features my UI solution had ( I understand why you wouldn't, but thought to at least mention them explicitly):

a. A loading screen between reloads. It is more cohesive I think, and if you are in dark mode and the original website is very white, this prevents an unpleasant flicker I always dread when using the reader mode on my iPhone at night. Might be less of an issue as dark mode extensions become more common.

b. Allow going to next/prev page via a keyboard shortcut without opening reader. This is indeed unrelated to the reader functionality, but I just find it very useful on many websites and the logic is already built in. Might be a nice extra.

rNeomy commented 1 year ago

Looks good.

but I would set opening links in the reader as on by default,

I aim to keep the default permissions to a minimum. However, we require the "*" host permission to manage the next page load. Based on your feedback, I've included a new tip to let users know they can enable this feature.

Regarding the other UI enhancements, let's roll out this update to gather feedback. We can always expand on it in the future.