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.

Sign up for GitHub

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

Jump to bottom

refactor: KeyMsg -> KeyPressMsg #600

Closed
wants to merge 3 commits into from
Closed

refactor: KeyMsg -> KeyPressMsg #600

wants to merge 3 commits into from

Conversation

bashbunni
Copy link
Member

@bashbunni bashbunni commented Aug 22, 2024 •
edited
Loading

Cherry picked a single commit with changes from alpha branch with latest master

@bashbunni
Copy link
Member Author

Looking into why the test is failing

@bashbunni bashbunni self-assigned this Aug 22, 2024
@bashbunni bashbunni marked this pull request as draft August 22, 2024 18:38
@bashbunni
Copy link
Member Author

Tests aren't passing on this branch - terminal color codes don't work. Need to fix that before it's ready :)

@bashbunni bashbunni added this to the v2.0.0 milestone Aug 23, 2024
@bashbunni bashbunni removed the v2-exp label Aug 23, 2024
aymanbagabas
aymanbagabas reviewed Aug 23, 2024
View reviewed changes
Copy link
Member

@aymanbagabas aymanbagabas left a comment

Choose a reason for hiding this comment

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

LGTM

viewport/viewport.go
if !m.MouseWheelEnabled || msg.Action != tea.MouseActionPress {
break
}
case tea.MouseWheelMsg:
switch msg.Button {
Copy link
Member

Choose a reason for hiding this comment

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

With charmbracelet/bubbletea#1111, this is going to be msg.Button()

@bashbunni bashbunni marked this pull request as ready for review August 26, 2024 18:28
@bashbunni
Copy link
Member Author

@aymanbagabas do you have any thoughts on the colour issue? It seems the v2-exp lipgloss branch loses support for ansi colour codes as you can see when you run the tests on this branch. It doesn't handle ints like it used to for lipgloss.Color

@bashbunni bashbunni assigned aymanbagabas and unassigned bashbunni Sep 4, 2024
@bashbunni
Copy link
Member Author

on Bubble Tea v2-exp it's now KeyPressMsg.Code not KeyPressMsg.Runes. Need to update that here before merging

@bashbunni
Copy link
Member Author

swapping out []rune with rune in bubbles is also needed based on bubble tea's v2-exp branch

@aymanbagabas
Copy link
Member

fwiw, i've pushed these changes to v2-exp

@bashbunni
Copy link
Member Author

bashbunni commented Sep 10, 2024 •
edited
Loading

@aymanbagabas kk sounds good! I'll close this PR then

@bashbunni bashbunni closed this Sep 10, 2024
@caarlos0 caarlos0 deleted the key-changes-cherry-picked branch September 12, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aymanbagabas aymanbagabas aymanbagabas left review comments

@meowgorithm meowgorithm Awaiting requested review from meowgorithm meowgorithm is a code owner

Assignees

@aymanbagabas aymanbagabas

Labels
None yet
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@bashbunni @aymanbagabas