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: indexing and autoread retoggle #115

Merged

Conversation

coolsloth55
Copy link
Contributor

@coolsloth55 coolsloth55 commented Jan 4, 2025

Mark Read

demo-read

Show Read Version

demo-mark-show

Indexing For AutoRead

demo-indexing

Misc

I didn't notice this first time around but page down is actually f/pgdn page down. Switched to F as it's annoying if you favorite and it moves your screen.

@coolsloth55
Copy link
Contributor Author

coolsloth55 commented Jan 4, 2025

@Axlefublr @guyfedwards

Added demo gifs for convenience but feel free to pull and check out flow to see if anything is off. Second part of #113 will have to come in separate pr.

@Axlefublr
Copy link

looks awesome! :3

@coolsloth55
Copy link
Contributor Author

Found a issue with navigating reverse with autoread and resolved that. There's still two slight navigation quirks that may be fine ::

prev with autoread just brings next item (index) ::

demo-weird-flow

Quit after marking last index as read looses cursor currently ::

demo-mark-weird-flow

@coolsloth55
Copy link
Contributor Author

Would it be reasonable to ignore trying to mark un-read within post after it was just done so? If not navigation might have to be switched to work by skipping over posts if show read isn't turned on. Alternatively maybe keep a backup so it could be re-added to list.

For example currently if autoread is on or mark read manually each post essentially drops from navigation list when using l/h.

@Axlefublr
Copy link

@coolsloth55 sorry I don't think I understand what you mean

while inside of post, if you mark it as read and then immediately as unread — the possibly problematic / annoying to implement part (right?)

if read posts are hidden, you suggest to skip over posts — not sure which posts you mean to skip

the last sentence I don't get also

@coolsloth55
Copy link
Contributor Author

while inside of post, if you mark it as read and then immediately as unread — the possibly problematic / annoying to implement part (right?)

Design decision, there's no indication currently when viewing the post whether it's been read. Similar to #112 you might unknowingly un-read a post. A navigation side affect is a follows since you can only mark read currently and not undo ::

launch nom
don't show read
view item-2
mark item-2 as read
can only navigate to prev (item-1) or next (item-3)
can't navigate back to item-2

If item-2 is read do we want allow undoing read in post view?

@Axlefublr
Copy link

ohhhhh I see! yeah, now that you mention it, it is a bit missing ux to not know whether the opened post is marked as read or not
I didn't realize it, but now I would love to have some sort of marker

having the marker will solve the ambiguity, but it then implies being able to undo it. it's not necessary, but would be surprising behavior if you see a toggle, and can only toggle it to one side

@coolsloth55
Copy link
Contributor Author

coolsloth55 commented Jan 6, 2025

I didn't realize it, but now I would love to have some sort of marker

Might be easy to prefix/postfix post title with something like ✓ - <title>. Definitely not glamorous but a clear indication.

having the marker will solve the ambiguity, but it then implies being able to undo it. it's not necessary, but would be surprising behavior if you see a toggle, and can only toggle it to one side

Toggle definitely make sense then especially since it's "manual" and matches list view read.

@Axlefublr
Copy link

yes! I like the idea of a prefix, (probably) easy to implement and does the job
I like that checkmark symbol, too

@coolsloth55
Copy link
Contributor Author

demo-indicator-and-fix-cursor

@Axlefublr
Copy link

omg that's so nice :D

@guyfedwards guyfedwards marked this pull request as ready for review January 6, 2025 10:17
Copy link
Owner

@guyfedwards guyfedwards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @coolsloth55, couple of small comments but the core of it looks good. 🎉

internal/commands/commands.go Outdated Show resolved Hide resolved
internal/commands/key.go Outdated Show resolved Hide resolved
internal/commands/viewport.go Outdated Show resolved Hide resolved
internal/commands/viewport.go Outdated Show resolved Hide resolved
internal/commands/viewport.go Outdated Show resolved Hide resolved
internal/commands/viewport.go Outdated Show resolved Hide resolved
Copy link
Owner

@guyfedwards guyfedwards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @coolsloth55, couple of small comments but the core of it looks good. 🎉

@coolsloth55 coolsloth55 changed the title Fix/indexing and autoread retoggle fix: indexing and autoread retoggle Jan 6, 2025
@guyfedwards guyfedwards merged commit 283ee62 into guyfedwards:master Jan 7, 2025
2 checks passed
@guyfedwards
Copy link
Owner

thanks @coolsloth55

@Axlefublr
Copy link

I'm using this now and WOW, what a pleasure! this RSS reader was already way better than any alternatives I found, but thanks to both of you it has become even more top tier. phenomenal!!

@guyfedwards
Copy link
Owner

Appreciate the comment @Axlefublr, thank you for all your feedback and bug reports. Shout out and thanks to @coolsloth55 for all their contributions!

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.

3 participants