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

Selected item out of sync #118

Closed
Hacksore opened this issue May 29, 2020 · 3 comments
Closed

Selected item out of sync #118

Hacksore opened this issue May 29, 2020 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@Hacksore
Copy link
Contributor

The selected item in the tree seems to be out of sync and typically not the item I clicked on.

It seems like a bug to me but opening an issue to track this nevertheless and to get some feedback from others.

gif

@berzniz
Copy link
Owner

berzniz commented Jun 2, 2020

Thanks for reporting. It seems like GitHub doesn't scroll all the way when navigating using links (URL change), so it goes out of sync.

If anyone wants to try and fix it, I'll appreciate a PR

@berzniz berzniz added the help wanted Extra attention is needed label Jun 2, 2020
@aramperes
Copy link
Contributor

I'd love to help with this. My understanding is that the tree highlights the first file visible in the viewport.

const nextVisibleElement = diffElements.find(isElementVisible)
if (nextVisibleElement !== visibleElement) {
this.setState({
visibleElement: nextVisibleElement
})
}

To fix this, I would suggest prioritising an element with the :target pseudo-class, and check if it's in the viewport. If it's not, default back to the current logic (first visible file).

The behaviour would feel more natural: clicking on the tree item would select the right one, and scrolling up the page would scroll up in the tree as well.

aramperes added a commit to aramperes/github_pr_tree that referenced this issue Aug 17, 2020
berzniz added a commit that referenced this issue Sep 19, 2020
#118 Fix tree desync by taking :target into consideration
@berzniz
Copy link
Owner

berzniz commented Sep 20, 2020

Will be in included in 1.0.32

@berzniz berzniz closed this as completed Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants