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

WIP: feat(position): Add function to place an overlay #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

buztard
Copy link

@buztard buztard commented Aug 23, 2022

This is just a quick POC I made the other day. Needs cleanup, proper testing, relative placement using lipgloss.Position and the ability to place the overlay outside of the background box, but so far it works as expected.

overlay

@buztard
Copy link
Author

buztard commented Aug 23, 2022

Addresses #65 and charmbracelet/bubbletea#79

@muesli muesli added the enhancement New feature or request label Oct 5, 2022
@Chillance
Copy link

This is quite neat and useful. Are you @buztard working on improving this for merging?

@mrusme
Copy link

mrusme commented Dec 30, 2022

What would need to be improved here for this to become mergable? @muesli @meowgorithm It would be super useful to have this for the time being until an actual compositor becomes available.

mrusme added a commit to mrusme/neonmodem that referenced this pull request Dec 31, 2022
@coxley
Copy link

coxley commented Jan 3, 2023

Ooo.

@meowgorithm
Copy link
Member

meowgorithm commented Jan 3, 2023

What would need to be improved here for this to become mergable? @muesli @meowgorithm It would be super useful to have this for the time being until an actual compositor becomes available.

Hey, Marius! If this is something you need immediately I'd suggest simply moving the code here to a separate repo (or adding directly to your project). It's just string manipulation in the end and should be pretty straightforward.

Otherwise, we're could potentially merge this after vetting performance a bit and, more importantly, making sure it works properly with double-width runes (which I'm sure you can appreciate).

@mrusme
Copy link

mrusme commented Jan 9, 2023

adding directly to your project

Hey Christian! Did exactly that, works like a Charm! Had to copypasta a few internals though, but it's okay for now. Would nevertheless be cool to have it officially available. :-)

@meowgorithm
Copy link
Member

Noted! In the meantime, @mrusme, I'd love to hear if you encounter any problems with the implementation (or if everything is awesome).

@Chillance
Copy link

Chillance commented May 22, 2023

@mrusme The overlay one is nifty. Any idea how to make it work with https://github.com/lrstanley/bubblezone markers? @meowgorithm Overlay code works great, but not using those bubblezone markers from what I can tell.

Edit: After further test, it seems that essentially markers won't work properly when using the Overlay functionality. I do zone.Mark and zone.Scan, and it works. As soon as I use the .PlaceOverlay function, those markers don't work properly anymore.

@0xsbeem
Copy link

0xsbeem commented May 5, 2024

Is this functionality going to be merged into lipgloss? Or is it going to be tackled as part of the "larger initiative" mentioned here: charmbracelet/bubbletea#169 (comment)? @meowgorithm

@mrusme The overlay one is nifty. Any idea how to make it work with https://github.com/lrstanley/bubblezone markers? @meowgorithm Overlay code works great, but not using those bubblezone markers from what I can tell.

Edit: After further test, it seems that essentially markers won't work properly when using the Overlay functionality. I do zone.Mark and zone.Scan, and it works. As soon as I use the .PlaceOverlay function, those markers don't work properly anymore.

I've been using this .PlaceOverlay functionality with bubblezone markers and it's been working fine for me.

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

7 participants