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

feat: support custom styles for cells #397

Closed
wants to merge 7 commits into from

Conversation

hedhyw
Copy link

@hedhyw hedhyw commented Jul 23, 2023

Add optional RenderCell function. If it is not specified then Styles.Cell.Render is used. The changes are not breaking.

In action:
image

Relates: #246

Copy link
Member

@bashbunni bashbunni left a comment

Choose a reason for hiding this comment

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

Looks good! Since it's a change to the API, I'll discuss with the team to confirm naming is all good.

Copy link
Member

@bashbunni bashbunni left a comment

Choose a reason for hiding this comment

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

Actually I'm not sure this is behaving as expected. Just tested with our table example in Bubble Tea and found it broke existing rendering. Other rows kept the Selected styles just fine. Let me know your thoughts :)

Here's what I added to highlight Tokyo:

	s.RenderCell = func(value string, rowID, columnID int) string {
		if strings.Contains(value, "Tokyo") {
			return lipgloss.NewStyle().Foreground(lipgloss.Color("42")).Render(value)
		}
		return value
	}

Here's what the TUI looked like:
image

@bashbunni bashbunni added the enhancement New feature or request label Jul 25, 2023
@hedhyw
Copy link
Author

hedhyw commented Jul 26, 2023

Hi @bashbunni , thanks for the review!

The issue is due to Control Sequence Introducer with Reset 0 (it resets all styles) in termenv which is used by lipgloss: https://github.com/muesli/termenv/blob/346688723df2b59a0bd02a31376707ef51042ed5/style.go#L56
There is no simple solution for this. It is not originally designed for full support of nested styles...

So currently, the following code breaks colors:

row.Render(
	lipgloss.JoinHorizontal(
		cell1.Render(), // It clears styles...
		cell2.Render(),
		cell3.Render(),
	),
)

So that is why I added RenderCell, not GetCellStyle, to have more control over it. Ideally, all styles should be handled in RenderCell. I think maybe let's add an instance of a table or selectedRowID to the arguments of the function. rowID, columnID, and selectedRowID can be combined in one struct. I changed the implementation to be more convenient, please check the next comment with an example of usage.

Example:

s.RenderCell = func(model table.Model, value string, position table.Position) string {
	if position.RowID == model.Cursor() {
		return s.Cell.Copy().
			Foreground(lipgloss.Color("229")).
			Background(lipgloss.Color("57")).
			Render(value)
	}

	return s.Cell.Copy().Foreground(lipgloss.Color("21")).Render(value)
}

Reference:

What do you think?

@hedhyw
Copy link
Author

hedhyw commented Jul 26, 2023

@hedhyw hedhyw requested a review from bashbunni July 26, 2023 06:17
@hedhyw
Copy link
Author

hedhyw commented Sep 4, 2023

@bashbunni Hi, could you please check the review changes?

@hedhyw
Copy link
Author

hedhyw commented Nov 5, 2023

@bashbunni @maaslalani Hi, Is there any chance this to be merged? Thank you!

Example of usage:
https://gist.github.com/hedhyw/2fae751dd233d229ed3cc5209007f009#file-bubble_table_example-go-L176

@qvistgaard
Copy link

Are there any updates on this PR. I could really use the ability to apply specific cell styles in my project

@maaslalani
Copy link
Member

Hey! I really like this idea. The long term plan is to switch over to using Lip Gloss tables which should cover this use case:

Lip Gloss tables have a StyleFunc which takes the (row, col) and allows you to specify a style for each individual cell. So we would expose that on the table bubble once we switch over to using that.

@maaslalani
Copy link
Member

Thank you again for this contribution, I'm cleaning up some old PRs so I will close this for now but this feature will definitely be introduced once we switch over to the new Lip Gloss tables!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants