Skip to content
This repository has been archived by the owner on Dec 22, 2023. It is now read-only.

Implement horizontal scrolling #84

Merged
merged 5 commits into from
Nov 6, 2023

Conversation

janopae
Copy link

@janopae janopae commented Nov 5, 2023

Trelby uses a very complicated custom implementation for vertical scrolling, which can't be extended to work for horizontal scrolling.

In #72, there is an attempt to replace this custom implementation by using more of the toolkit's functionality. However, this is a huge projects that includes adjusting a lot of the rendering code and other parts of the application, which I'm not willing to invest right now.

This PR instead uses our wxWidgets toolkit for horizontal scrolling only, letting this implementation co-exist with the custom vertical scrolling implementation. #72 could later still be rebased on top of this in order to streamline the implementation and provide more native toolkit functionality also for vertical scrolling.

Horizontal scrolling is a necessary step towards implementing #20.

By putting the Control into a ScrolledWindow, we get scroll bars whenever the Control's container is smaller than its minimum size.

Vertical scrolling of the ScrolledWindow currently is deactivated, as we still have our custom implementation for vertical scrolling.

This, however, raises some issues: At first, the vertical scroll bar gets scrolled away along with the Screenplay Control while scrolling horizontally, as it is part of the contents of the ScrolledWindow.

Also, the hand-made scrollbar lacks some features, such as auto-hiding or the "precise scrolling mode" on Gtk.

Eventually, the custom scrolling implementation is probably going to get in our way when implementing  limburgher#20 anyway, so we should try to port vertical scrolling to the ScrolledWindow scrolling mechanism.
Before this commit, horizontal scrolling also moves the vertical scroll bar.
@janopae
Copy link
Author

janopae commented Nov 5, 2023

2 of the pipelines still fail, but this is a known issue. I consider this ready to merge.

I specifically implemented fd5c593 to make this comment not necessary
@janopae janopae marked this pull request as draft November 5, 2023 22:49
@janopae janopae marked this pull request as ready for review November 5, 2023 22:50
@janopae
Copy link
Author

janopae commented Nov 6, 2023

I turned making the current character visible while typing into a separate issue (#86) so this can be merged, because you won't run into that when you don't use horizontal scrolling, so none of the previous use cases will be affected.

@limburgher
Copy link
Owner

Wonderful, thank you!

@limburgher limburgher merged commit a9afa82 into limburgher:main Nov 6, 2023
2 of 4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants