Skip to content

refactor: cleanup and document Scrollbar - #5334

Merged
pajlada merged 36 commits into
Chatterino:masterfrom
Nerixyz:refactor/scrollbar
May 12, 2024
Merged

pajlada merged 36 commits into
Chatterino:masterfrom
Nerixyz:refactor/scrollbar

Conversation

@Nerixyz

@Nerixyz Nerixyz commented Apr 19, 2024

Copy link
Copy Markdown
Contributor

Working on #5329, I realized that Scrollbar needed some documentation and a rename of largeChange. Then I saw some unused functions and clang-tidy warnings, so I decided to refactor the whole class.

The commits and documentation should work on their own - if you're missing information, please point that out and I'll add more docs.

Since I added some documentation, I decided to add a doxygen target (if available), so I don't have to re-add that every time I want to generate documentation. It's not meant to be pretty - we could use breathe for that. But it's not really a priority since Chatterino isn't a library.

@pajlada pajlada added this to the Post-2.5.0 milestone Apr 19, 2024
@Pizzabelly

Copy link
Copy Markdown
Contributor

Nice. I could suggest renaming resetMaximum() to resetBounds(). Maybe also a note about getRelativeCurrentValue() being the current position adjusted to the bounds.

@Nerixyz Nerixyz mentioned this pull request Apr 24, 2024
4 tasks done
@pajlada

pajlada commented May 4, 2024

Copy link
Copy Markdown
Member

I added some tests for addHighlightsAtStart that run on current master, but fails with this PR

Comment thread src/widgets/Scrollbar.hpp
@dnsge

dnsge commented May 4, 2024

Copy link
Copy Markdown
Contributor

I have a couple more things that would be nice to refactor, see Nerixyz#22 :)

@pajlada
pajlada enabled auto-merge (squash) May 12, 2024 10:27
@pajlada
pajlada merged commit 8202cd0 into Chatterino:master May 12, 2024
@Nerixyz
Nerixyz deleted the refactor/scrollbar branch May 12, 2024 11:35
babicekmarek pushed a commit to babicekmarek/streamally-chatterino that referenced this pull request Sep 14, 2026
Co-authored-by: Rasmus Karlsson <rasmus.karlsson@pajlada.com>
Co-authored-by: Daniel Sage <sagedanielr@gmail.com>
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.

4 participants