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

Improve pagination for openqa-mon #160

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Improve pagination for openqa-mon #160

merged 1 commit into from
Aug 20, 2024

Conversation

grisu48
Copy link
Collaborator

@grisu48 grisu48 commented Aug 20, 2024

Add some minor improvements to the pagination feature:

  • Display page count from the beginning onwards
  • Add handlers for home, end, page up, page down and arrow left/right keys
  • Use case instead of nested ifs in the openqa-revtui key handler

Follow-up of #157, in preparation of a Release-Candidate

@grisu48
Copy link
Collaborator Author

grisu48 commented Aug 20, 2024

@ilmanzo please have a look, if you have time 🙂

@ilmanzo
Copy link
Contributor

ilmanzo commented Aug 20, 2024

Overall very good, left only a small comment for better readability :)

@ilmanzo
Copy link
Contributor

ilmanzo commented Aug 20, 2024

Nit: would be great to avoid a screen refresh/update when the page content does not actually change.

For example when you are on the first page and press Left or page up, then the screen flickers to update itself but it's really not needed. Maybe we can just use a dirty flag to signal the Update() function it's time to redraw or not ?

func (tui *TUI) PrevPage() {
	tui.Model.mutex.Lock()
	defer tui.Model.mutex.Unlock()
    if tui.currentPage > 0 {
	  tui.currentPage--
      // here we need to refresh
      tui.dirty=true
    }
}

but we can just bring this to another issue, possibly related also to #159

@grisu48
Copy link
Collaborator Author

grisu48 commented Aug 20, 2024

Nit: would be great to avoid a screen refresh/update when the page content does not actually change.

For example when you are on the first page and press Left or page up, then the screen flickers to update itself but it's really not needed. Maybe we can just use a dirty flag to signal the Update() function it's time to redraw or not ?

func (tui *TUI) PrevPage() {
	tui.Model.mutex.Lock()
	defer tui.Model.mutex.Unlock()
    if tui.currentPage > 0 {
	  tui.currentPage--
      // here we need to refresh
      tui.dirty=true
    }
}

but we can just bring this to another issue, possibly related also to #159

We can, but so far screen refreshes are cheap on a local terminal, so I didn't bothered much.

Add some minor improvements to the pagination feature:

* Display page count from the beginning onwards
* Add handlers for home, end, page up, page down and arrow left/right
  keys
* Use case instead of nested ifs in the openqa-revtui key handler
@grisu48 grisu48 merged commit 97ca052 into os-autoinst:main Aug 20, 2024
2 checks passed
@grisu48 grisu48 deleted the rc branch August 20, 2024 13:32
@grisu48 grisu48 mentioned this pull request Aug 21, 2024
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