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 scrolling issues #247

Merged
merged 1 commit into from
Oct 20, 2021
Merged

Fix scrolling issues #247

merged 1 commit into from
Oct 20, 2021

Conversation

blogh
Copy link
Collaborator

@blogh blogh commented Oct 14, 2021

The PR aims to solve some problems related to scrolling (#239)

@blogh blogh marked this pull request as draft October 14, 2021 14:35
@blogh
Copy link
Collaborator Author

blogh commented Oct 14, 2021

@dlax : this is far from finished but to you see a problem with how I handled scrolling ?

@dlax
Copy link
Member

dlax commented Oct 15, 2021

I'd suggest to only fix one problem per PR; i.e. avoid header refresh in interactive mode.
And submit another PR for scrolling (there, please explain in the commit message how things are fixed).

@blogh
Copy link
Collaborator Author

blogh commented Oct 15, 2021

thx for the input. will do when it's no longer "experimental"

@blogh blogh force-pushed the fix_scrolling_issues branch from 1326ef5 to a90a520 Compare October 18, 2021 12:47
@blogh
Copy link
Collaborator Author

blogh commented Oct 18, 2021

The refresh while pause thing was pushed in #248.

pgactivity/views.py Outdated Show resolved Hide resolved
Copy link
Member

@dlax dlax left a comment

Choose a reason for hiding this comment

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

The approach looks good overall.
It might be worth covering this in test_views.txt.

pgactivity/views.py Outdated Show resolved Hide resolved
pgactivity/views.py Outdated Show resolved Hide resolved
pgactivity/views.py Outdated Show resolved Hide resolved
@blogh
Copy link
Collaborator Author

blogh commented Oct 18, 2021

Will add / fix tests later.

pgactivity/types.py Outdated Show resolved Hide resolved
pgactivity/views.py Outdated Show resolved Hide resolved
pgactivity/views.py Outdated Show resolved Hide resolved
Copy link
Member

@dlax dlax left a comment

Choose a reason for hiding this comment

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

Thanks for the tests, these look good.

Still a formatting issue, see CI "lint" job.

Then please squash this all, and I'll give a final look.

tests/test_views.txt Outdated Show resolved Hide resolved
tests/test_views.txt Outdated Show resolved Hide resolved
tests/test_views.txt Show resolved Hide resolved
@dlax
Copy link
Member

dlax commented Oct 19, 2021

Also, if you could write a more informative commit message than "Handle scrolling", that'd be great!

@blogh blogh force-pushed the fix_scrolling_issues branch 3 times, most recently from 6270d09 to e67cf4c Compare October 19, 2021 10:01
tests/test_views.txt Outdated Show resolved Hide resolved
pgactivity/views.py Outdated Show resolved Hide resolved
tests/test_views.txt Outdated Show resolved Hide resolved
@blogh blogh force-pushed the fix_scrolling_issues branch from e67cf4c to e07ce46 Compare October 20, 2021 08:32
When the line count overflows outside the screen, we can now properly
scroll down. If the focus reaches 4/5 of the available lines on screen,
the first line is removed and we display a new line at the bottom. When
we reach the last line, pressing down will focus on the fist line again.
Likewise, if the up key is pressed when the focus is on the first line,
we focus on the last one.
@blogh blogh force-pushed the fix_scrolling_issues branch from e07ce46 to 3d5d172 Compare October 20, 2021 08:59
Copy link
Member

@dlax dlax left a comment

Choose a reason for hiding this comment

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

Excellent, thanks!

@blogh blogh marked this pull request as ready for review October 20, 2021 09:44
@blogh blogh merged commit 7ff0dcf into dalibo:master Oct 20, 2021
@blogh blogh deleted the fix_scrolling_issues branch September 16, 2022 12:48
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