tslocum/cview
tslocum
/
cview
2
4
Fork 4
Code Issues 17 Pull Requests 1 Releases Activity

WIP: add compact flag #83

Draft
caninodev wants to merge 1 commits from caninodev/cview:compact_list into master
pull from: caninodev/cview:compact_list
merge into: tslocum:master
Conversation 2 Commits 1 Files Changed 1 +23 -2
1 changed files with 23 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
list.go
View File

@ -194,6 +194,10 @@ type List struct {
// The height of the list the last time it was drawn.
height int
// If true, will render each ListItem without blank lines in between. This flag
tslocum commented 2021-12-18 23:09:26 +00:00
Review
Copy Link

Could I suggest widening the scope of this feature slightly? A compact list could be a list which, when no list items have secondary text set, will not add a blank line for the missing text, and will render as though ShowSecondaryText were false. When an item with secondary text is added, it is as though ShowSecondaryText were set to true. I also believe compact should default to true.

Could I suggest widening the scope of this feature slightly? A compact list could be a list which, when no list items have secondary text set, will not add a blank line for the missing text, and will render as though `ShowSecondaryText` were false. When an item with secondary text is added, it is as though `ShowSecondaryText` were set to true. I also believe compact should default to true.
caninodev commented 2021-12-21 03:11:21 +00:00
Review
Copy Link

So to make sure I understand your suggestion:

If a list's list items do not have SecondaryText defined, then the list should be rendered compact by default.

However, should a list's list items have SecondaryText defined, then list should not be rendered compact by default?

Or do you mean to say that the list, in both conditions, be rendered compact?

So to make sure I understand your suggestion: If a list's list items do not have `SecondaryText` defined, then the list should be rendered compact by default. However, should a list's list items have `SecondaryText` defined, then list should _not_ be rendered compact by default? Or do you mean to say that the list, in both conditions, be rendered compact?
tslocum commented 2021-12-21 20:54:36 +00:00
Review
Copy Link

The prior is what I meant. Thanks!

The prior is what I meant. Thanks!
// is ignored if `showSecondaryText` is true.
compact bool
sync.RWMutex
}
@ -209,6 +213,7 @@ func NewList() *List {
selectedTextColor: Styles.PrimitiveBackgroundColor,
scrollBarColor: Styles.ScrollBarColor,
selectedBackgroundColor: Styles.PrimaryTextColor,
compact: false,
}
l.ContextMenu = NewContextMenu(l)
@ -519,6 +524,15 @@ func (l *List) SetDoneFunc(handler func()) {
l.done = handler
}
// SetCompactList sets the flag that determines whether a blank line is drawn between
// each ListItem. This flag will be ignored if `showSecondaryText` is true.
func (l *List) SetCompactList(compact bool) {
tslocum commented 2021-12-18 23:02:06 +00:00
Review
Copy Link

Please rename as SetCompact

Please rename as `SetCompact`
l.Lock()
defer l.Unlock()
l.compact = compact
}
// AddItem calls InsertItem() with an index of -1.
func (l *List) AddItem(item *ListItem) {
l.InsertItem(-1, item)
@ -799,7 +813,11 @@ func (l *List) updateOffset() {
}
} else {
if l.currentItem-l.itemOffset >= h {
l.itemOffset = l.currentItem + 1 - h
if l.compact {
tslocum commented 2021-12-18 23:05:31 +00:00
Review
Copy Link

To reduce duplication, please update the implementation to simply skip l.showSecondaryText when compact mode is enabled and there is no secondary text in the list. For instance, the condition:

} else if l.showSecondaryText {

could be updated to take l.compact into account. This should allow this feature to be added without adding any additional special cases outside of skipping l.showSecondaryText

To reduce duplication, please update the implementation to simply skip l.showSecondaryText when compact mode is enabled and there is no secondary text in the list. For instance, the condition: ` } else if l.showSecondaryText {` could be updated to take l.compact into account. This should allow this feature to be added without adding any additional special cases outside of skipping l.showSecondaryText
l.itemOffset = l.currentItem - h
} else {
l.itemOffset = l.currentItem + 1 - h
}
}
}
@ -978,7 +996,10 @@ func (l *List) Draw(screen tcell.Screen) {
RenderScrollBar(screen, l.scrollBarVisibility, scrollBarX, y, scrollBarHeight, len(l.items), scrollBarCursor, index-l.itemOffset, l.hasFocus, l.scrollBarColor)
y++
// Compact List
if !l.compact {
y++
}
if y >= bottomLimit {
break