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.

Sign up for GitHub

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

Jump to bottom

VIM Visual Mode #80

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

VIM Visual Mode #80

wants to merge 14 commits into from

Conversation

Enoumy
Copy link
Contributor

@Enoumy Enoumy commented May 26, 2023

Addresses #79

Pressing Shift+V or v in the manga page will toggle something similar to vim's visual mode. (It's not fully 1:1 with vim's visual mode as you can't select things horizontally).

This lets you select a range of manga easily + it has the side effect of letting you select all manga chapters with ggvG.

Testing

Manual testing by runnging go build and then ./mangadesk.

@Enoumy
Copy link
Contributor Author

Enoumy commented May 26, 2023

Oh whoops, sorry, I'm a bit of a github newbie too. I think this diff also includes the commits from the guest mode pr. Is this fine and just gets resolved whenever the other pr is merged? Still sad that the diff from the guest feature shows up in your review though. Will be moving this pr to a draft in the meantime.

@Enoumy Enoumy marked this pull request as draft May 26, 2023 01:07
@darylhjd
Copy link
Owner

darylhjd commented Jul 4, 2023

You will likely need to rebase once I merge your other PR to the main branch.

@Enoumy Enoumy marked this pull request as ready for review July 29, 2023 16:53
@darylhjd
Copy link
Owner

Sorry for the inaction regarding this PR, have been busy.

Seems like this branch is currently not rebased on main. I will do that and review it soon.

Thanks!

@darylhjd
Copy link
Owner

Please add new keybindings to the README.

@darylhjd darylhjd linked an issue Jan 27, 2024 that may be closed by this pull request
[Feature Request] VIM Visual Mode for Selecting Manga #79
Open
darylhjd
darylhjd reviewed Jan 27, 2024
View reviewed changes
app/ui/page_inputs.go Outdated Show resolved Hide resolved
@Enoumy
Copy link
Contributor Author

Enoumy commented Jan 31, 2024

Sorry for the inaction regarding this PR, have been busy.

No worries! Thanks for reviewing! Thanks for rebasing too!

Please add new keybindings to the README.

Added! Lmk if reasonable wording!

I've also included your changes and re-manually tested by building + running and testing that selecting in visual mode still works manually.

Thanks!

@darylhjd
Copy link
Owner

darylhjd commented Mar 5, 2024

im not too familiar with visual mode on VIM, so i am not sure what it does and how the behaviour translates to mangadesk.

i guess it works something like text selection, just that it works with rows here. do correct me if im wrong.

from my testing, it works well, but there are a few quirks:

  1. Starting visual mode (pressing 'v' once) provides no indication that the mode is active, since the current row isn't highlighted green immediately. only after moving up or down once is there an indication.
    • Ideally, the indication that the mode is active should be shown (i.e. current row where mode was activated is highlighted).
  2. If there are currently selected chapters (e.g. from ctrlE toggling), entering visual mode deselects all of them.
    • I am okay with this behaviour as we can treat it as previously highlighting something, and then now changing to highlighting something else. But the behaviour of this could be documented in the README, perhaps as a small note in the keybindings table. I can foresee that this behaviour might be unexpected to some users.

darylhjd
darylhjd reviewed Mar 5, 2024
View reviewed changes
app/ui/page_inputs.go
@@ -189,6 +189,12 @@ func (p *MangaPage) setHandlers(cancel context.CancelFunc) {
return event
})

p.Table.SetSelectionChangedFunc(func(row, _column int) {
Copy link
Owner

Choose a reason for hiding this comment

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

A comment on what this binding does would be useful. See line 182 of same file for example.

darylhjd
darylhjd reviewed Mar 5, 2024
View reviewed changes
app/ui/page_inputs.go
@@ -234,6 +242,37 @@ func (p *MangaPage) ctrlAInput() {
p.markAll()
}

func (p *MangaPage) selectRange(from, to int) {
Copy link
Owner

Choose a reason for hiding this comment

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

comment for this function and the one below would also be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darylhjd darylhjd darylhjd left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] VIM Visual Mode for Selecting Manga
2 participants
@Enoumy @darylhjd