Skip to content

feat: apply loading message delay for directories#2410

Merged
joelim-work merged 7 commits intogokcehan:masterfrom
joelim-work:preview-timer
Mar 4, 2026
Merged

feat: apply loading message delay for directories#2410
joelim-work merged 7 commits intogokcehan:masterfrom
joelim-work:preview-timer

Conversation

@joelim-work
Copy link
Collaborator

@joelim-work joelim-work commented Mar 3, 2026

This PR changes the way delays are handled when displaying the loading... message.

dir and reg objects contain a loadTime variable, which conceptually represents the time the object was last created or updated. For objects that represent real directories/previews, this is used to check for modifications by comparing against mtime. For placeholder objects representing directories/previews that are in the process of being loaded, this can be used to measure how long it has been loading, and whether or not enough time has passed to display loading... without causing flickering.

Copy link
Collaborator

@CatsDeservePets CatsDeservePets left a comment

Choose a reason for hiding this comment

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

Scrolling through directories feels much smoother now.

ui.go Outdated
switch {
case reg.loading:
if previewLoading {
if time.Since(reg.loadTime) > 100*time.Millisecond {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting. I never noticed before, but volatile previews only ever displayed loading... once, even though they have to load every time.

This is fixed now.

Copy link
Collaborator Author

@joelim-work joelim-work Mar 4, 2026

Choose a reason for hiding this comment

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

It is because of this part of the code, which marks the preview as reloading entirely from scratch if it is volatile:

if volatile && r.volatile {
	if !gOpts.preload {
		r.loadTime = time.Now()
		r.loading = true
	}
	nav.previewChan <- path
}

One thing I dislike about the initial implementation of previews is that it conflates 'loading' with 'displaying'. The preload option separates these two steps:

  • Loading is performing any prerequisite work (e.g. generating thumbnails from a video), and then loading the preview into a reg object.
  • Displaying is taking that reg object and physically displaying it in the UI. For text and sixel previews, the preview data is stored directly in the reg object, but volatile previews (e.g. Ueberzug images) have a flag set which then delegates the actual work to the previewer script.

This does mean now that the loading... message now only applies for the loading step, but in theory the displaying step should be fast and not require such a message.

@joelim-work joelim-work added the new Pull requests that add new behavior label Mar 4, 2026
@joelim-work joelim-work added this to the r42 milestone Mar 4, 2026
@joelim-work joelim-work added the fix Pull requests that fix existing behavior label Mar 4, 2026
@joelim-work joelim-work merged commit fd2babd into gokcehan:master Mar 4, 2026
32 checks passed
@joelim-work joelim-work deleted the preview-timer branch March 4, 2026 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix existing behavior new Pull requests that add new behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add delay before showing directory loading message

3 participants