Skip to content

Add option to highlight entire selected row of a List item#220

Merged
rivo merged 3 commits intorivo:masterfrom
ardnew:list-spanhighlight
Mar 8, 2019
Merged

Add option to highlight entire selected row of a List item#220
rivo merged 3 commits intorivo:masterfrom
ardnew:list-spanhighlight

Conversation

@ardnew
Copy link
Copy Markdown
Contributor

@ardnew ardnew commented Jan 3, 2019

This implements the option requested in #219

@rivo
Copy link
Copy Markdown
Owner

rivo commented Jan 24, 2019

Generally ok with this. But could you please rename the member variable (and the function accordingly) to highlightTextOnly? I find spanSelectedBackgroundColor confusing.

GitHub says there are merge conflicts. You might want to pull in the latest changes before resubmitting.

@rivo
Copy link
Copy Markdown
Owner

rivo commented Jan 24, 2019

You know, I've been thinking about this some more and I think this should not be the default. It will look quite odd on very wide screens with little text on a line.

So I think this should be called highlightFullLine instead and one can then use SetHighlightFullLine(true) to activate it.

@rivo
Copy link
Copy Markdown
Owner

rivo commented Feb 14, 2019

Do you still want to make the proposed change? (It seems that you'll need to resolve a merge conflict, too, as the file has changed in the meantime.)

@ardnew
Copy link
Copy Markdown
Contributor Author

ardnew commented Feb 20, 2019

finally resolved conflict and updated the field name as requested, please pull when ready

@ardnew
Copy link
Copy Markdown
Contributor Author

ardnew commented Feb 21, 2019

You know, I've been thinking about this some more and I think this should not be the default. It will look quite odd on very wide screens with little text on a line.

Yeah, I suppose in that case it might look odd. Is that really the most common case though?

To me it seems like the current implementation diverges from widespread convention. I can't think of any list in any application where only the text is highlighted (all context menus, dropdown menus, list boxes, combo boxes in macOS, Windows, Gnome, and KDE; all list controls in Chrome and Firefox).

I realize these are graphical applications, but why would the convention be any different?

EDIT:
The Linux kernel's ncurses config tool make menuconfig is an example where only text is highlighted, but this is more of a tree view than a list.

@rivo rivo merged commit c16128c into rivo:master Mar 8, 2019
@rivo
Copy link
Copy Markdown
Owner

rivo commented Mar 8, 2019

I don't know. I just tried it on demos/list/main.go and it looks odd to me. And I don't even have a huge screen (working on a 13 inch laptop).

But if there are actual complaints from more users, I'm willing to rethink this.

DomenicoSola pushed a commit to DomenicoSola/tview that referenced this pull request Oct 19, 2022
DomenicoSola pushed a commit to DomenicoSola/tview that referenced this pull request Oct 19, 2022
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