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 after gocui update #473

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

Conversation

mark2185
Copy link
Contributor

@mark2185 mark2185 commented Sep 6, 2023

This fixes the scrolling caused by a regression in #447 .

It still has some issues regarding highlighting rows, but I'm not overtly interested in diving into gocui given that it's not actively maintained. At least it enables the user to scroll again.

@vinkmr
Copy link

vinkmr commented Sep 21, 2023

Built using go1.21.0 and tested on Ubuntu 20.04. Unfortunately the scrolling bug is still there. :(

@mark2185
Copy link
Contributor Author

Ah, I see I've only fixed it for the Image Details pane, thanks.

@mark2185
Copy link
Contributor Author

mark2185 commented Oct 3, 2023

I took the liberty of rebasing onto #478 , give it another spin @vinkmr .

@vinkmr
Copy link

vinkmr commented Oct 4, 2023

Yup. Scroll seems to work for me now.

@marcusramberg
Copy link

Would be great to see a new release with this fix, as it severely limits the usability of dive for nix-based images :)

@glensc
Copy link

glensc commented Jan 16, 2024

To run this branch

$ gh repo clone https://github.com/wagoodman/dive         
Cloning into 'dive'...
remote: Enumerating objects: 5334, done.
remote: Counting objects: 100% (1258/1258), done.
remote: Compressing objects: 100% (315/315), done.
remote: Total 5334 (delta 1007), reused 1110 (delta 932), pack-reused 4076
Receiving objects: 100% (5334/5334), 5.05 MiB | 5.26 MiB/s, done.
Resolving deltas: 100% (3500/3500), done.

$ cd dive 

$ gh co 473           
From https://github.com/wagoodman/dive
 * [new ref]         refs/pull/473/head -> fix/scrolling-contents
Switched to branch 'fix/scrolling-contents'

$ go run main.go php:7.2-fpm

@MauriceArikoglu
Copy link

@wagoodman @mark2185 whats missing for this PR to get merged? Want to offer help 🤝

@mehmetumit
Copy link

Seems like these lines need to be fixed in order to pass ci check

runtime/ui/view/layer.go:298:14: Error return value of `v.vm.Update` is not checked (errcheck)
                v.vm.Update(v.constrainedRealEstate)
                           ^
runtime/ui/viewmodel/layer_set_state.go:42:26: func `(*LayerSetState).height` is unused (unused)
func (vm *LayerSetState) height() int {
                         ^
runtime/ui/view/layer.go:153:17: func `(*Layer).height` is unused (unused)
func (v *Layer) height() uint {
                ^

@black-snow
Copy link

Please release this - super annoying bug.

@ruffsl
Copy link

ruffsl commented Apr 8, 2024

I just had to rebase this PR on top of the current head ( 925cdd8 ) of main branch to work around something similar to this:

Thanks for the patch!

@MauriceArikoglu
Copy link

MauriceArikoglu commented Apr 8, 2024

Any update? @wagoodman
@mehmetumit @mark2185 can you fix the mentioned ci issues?

@LaTrissTitude
Copy link

Would be great for this fix to reach the next release

@black-snow
Copy link

Sadly, the logs have expired already.

@pov1ba pov1ba mentioned this pull request May 2, 2024
@pov1ba
Copy link

pov1ba commented May 2, 2024

You can still see what was the issue here. https://github.com/wagoodman/dive/actions/runs/6391362920?pr=473

I made another pull request based on this one here, #520, that should hopefully pass all the checks. I just need someone to review and approve it now.

Will be nice if we can finally have this fix in new versions.

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.

None yet