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

Individual message rendering overhaul #451

Closed
wants to merge 26 commits into from

Conversation

Retropaint
Copy link
Contributor

Closes #444

This is quite a hefty overhaul to how messages are rendered. Messages are now individual textViews rendered in a regular box, as opposed to one big textView. The benefits of this are described in the issue.

However, there is a concern I have for my implementation of this, which is the custom scrolling logic. Since tview boxes don't support scrolling, this has to be manually done. My implementation is very verbose but hopefully simple to understand and maintain. I had already tried a more 'compact' solution as seen in older commits but it was fragile in every sense of the word.

Note that this PR is strictly for the overhaul itself (made in the 'easiest' way possible) and doesn't have performance optimization. Attempting it was a time sink which I don't want to get into for now. I may add easy ones to implement into the PR if I find them.

Not sure if this is necessary to add, but I welcome any feedback on this system, particularly the scrolling logic (which I think can still be improved, I've just run out of ideas and time).

@Retropaint Retropaint changed the title Individual message overhaul Individual message rendering overhaul Oct 6, 2024
@Retropaint Retropaint marked this pull request as draft October 6, 2024 17:15
@Retropaint
Copy link
Contributor Author

Retropaint commented Oct 6, 2024

Marked as draft as I hadn't yet fixed detecting text line counts for those with words longer than the box width.

@Retropaint Retropaint marked this pull request as ready for review October 7, 2024 12:31
@Retropaint
Copy link
Contributor Author

Scrolling logic has improved, not as verbose and still just as stable. Also some optimization with not calling Render on off-screen messages and caching line counts (will need re-calculating for edited messages but thats for another time).

@Retropaint
Copy link
Contributor Author

There are a few line count padding issues to fix with code blocks but it requires changing renderer.go, and I don't want to add more files changed to this PR lest it have more conflicts down the line.

@ayn2op Please let me know what this PR needs before it can be merged. If it will take a while to review that's fine, just as long as it's merged or closed at some point.

@Retropaint
Copy link
Contributor Author

noticed a bug where messages won't render on their own without any input, since messagesText is now a Box and doesn't get implicitly called to redraw. Will fix a bit later.

@Retropaint Retropaint closed this Nov 22, 2024
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.

Handle messages individually
2 participants