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

allow scrolling past last entry / first entry #727

Merged
merged 6 commits into from
Oct 31, 2024

Conversation

nnyyxxxx
Copy link
Contributor

@nnyyxxxx nnyyxxxx commented Oct 2, 2024

watch the video

regular scrolling and tab scrolling NEEDS to be separated, without separation, we wont be able to properly handle subdir cases

Type of Change

  • New feature

Description

wraps the user to the top of the list or to the bottom of the list depending on where they are at
example:

2024-10-02-01-34-18.mp4

Copy link
Collaborator

@adamperkowski adamperkowski left a comment

Choose a reason for hiding this comment

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

Other functions here like scroll_up need to be refactored aswell.

tui/src/state.rs Outdated Show resolved Hide resolved
tui/src/state.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@lj3954 lj3954 left a comment

Choose a reason for hiding this comment

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

As mentioned in another comment, linutil core will panic on an empty tab array. Better error handling there may be a good idea, but the output of the function should still absolutely be guaranteed to not be empty; otherwise undefined behaviour could occur during UI rendering.

tui/src/state.rs Outdated Show resolved Hide resolved
tui/src/state.rs Outdated Show resolved Hide resolved
nnyyxxxx and others added 2 commits October 2, 2024 03:04
@adamperkowski
Copy link
Collaborator

You missed 2, @lj3954

tui/src/state.rs Show resolved Hide resolved
tui/src/state.rs Show resolved Hide resolved
Co-authored-by: Adam Perkowski <[email protected]>
tui/src/state.rs Outdated Show resolved Hide resolved
tui/src/state.rs Outdated Show resolved Hide resolved
tui/src/state.rs Outdated Show resolved Hide resolved
tui/src/state.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@adamperkowski adamperkowski left a comment

Choose a reason for hiding this comment

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

Thanks.

@lj3954
Copy link
Contributor

lj3954 commented Oct 2, 2024

You missed 2, @lj3954

No, those are wrong. The item list can be empty if a search term not matching anything is entered. You should get a panic with those changes applied in that case.

@lj3954
Copy link
Contributor

lj3954 commented Oct 2, 2024

Absolutely not. The item list can be empty, since a search can come up with no results. Only the tab list should be expected not to be empty.

@nnyyxxxx nnyyxxxx force-pushed the scrollpastlastentry branch from c0ec78b to 544cbd4 Compare October 2, 2024 07:41
@nnyyxxxx

This comment was marked as off-topic.

@nnyyxxxx

This comment was marked as off-topic.

@ChrisTitusTech ChrisTitusTech merged commit 1dcc3f3 into ChrisTitusTech:main Oct 31, 2024
1 check passed
@adamperkowski
Copy link
Collaborator

@nnyyxxxx i meant tall, not high. sorry.

@nnyyxxxx nnyyxxxx deleted the scrollpastlastentry branch November 1, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants