5
56
Fork
You've already forked gamja
22

Scroll active buffer into view #14

Open
petteri wants to merge 1 commit from petteri/gamja:scroll_sidebar into master
pull from: petteri/gamja:scroll_sidebar
merge into: emersion:master
emersion:master
emersion:unnecessary-const
emersion:eslint-extra
First-time contributor
Copy link

When the buffer list sidebar is long and does not fit to the page, scroll the current buffer selection into view.

When the buffer list sidebar is long and does not fit to the page, scroll the current buffer selection into view.

With React, querying the DOM directly is not something we should be doing. In addition, doing DOM manipulation like this every time the render function is called will not work reliable (for instance it'll scroll to the active buffer item when it's marked as read/unread, even if the user has scrolled away).

I think we need to use createRef() to store a reference to the item, and use the componentDidUpdate() lifecycle method to scroll if the item just became active.

With React, querying the DOM directly is not something we should be doing. In addition, doing DOM manipulation like this every time the render function is called will not work reliable (for instance it'll scroll to the active buffer item when it's marked as read/unread, even if the user has scrolled away). I think we need to use `createRef()` to store a reference to the item, and use the `componentDidUpdate()` lifecycle method to scroll if the item just became active.
Author
First-time contributor
Copy link

Oops, I didn't notice the automatic scrolling since I'm only using the keyboard. Thanks for the insight! I'll see if I can rework the patch.

Oops, I didn't notice the automatic scrolling since I'm only using the keyboard. Thanks for the insight! I'll see if I can rework the patch.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u scroll_sidebar:petteri-scroll_sidebar
git switch petteri-scroll_sidebar

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master
git merge --no-ff petteri-scroll_sidebar
git switch petteri-scroll_sidebar
git rebase master
git switch master
git merge --ff-only petteri-scroll_sidebar
git switch petteri-scroll_sidebar
git rebase master
git switch master
git merge --no-ff petteri-scroll_sidebar
git switch master
git merge --squash petteri-scroll_sidebar
git switch master
git merge --ff-only petteri-scroll_sidebar
git switch master
git merge petteri-scroll_sidebar
git push origin master
Sign in to join this conversation.
No reviewers
No labels
bug
enhancement
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
emersion/gamja!14
Reference in a new issue
emersion/gamja
No description provided.
Delete branch "petteri/gamja:scroll_sidebar"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?