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

Add scrolling to long commit messages #208

Merged
merged 1 commit into from
Aug 8, 2020

Conversation

cruessler
Copy link
Contributor

@cruessler cruessler commented Jul 20, 2020

  • Manually wrap commit messages
  • Add commands for navigation: focus commit message
  • Add scrolling to component
  • Add tests

@extrawurst
Copy link
Owner

@cruessler the windows build was broken due to #210 - once you rebase onto current master CI should be fine again

@cruessler
Copy link
Contributor Author

@extrawurst I updated the PR. Scrolling is not yet included, because I first wanted to ask whether the PR goes into the right direction. If so, adding the remaining parts should hopefully not be too difficult.

@extrawurst
Copy link
Owner

@cruessler from a first glance this looks like the way to go 👍

some nitpicks: let's not keep highlighting the top entry in the tree while the commit message is focused. I am also not sure about highlighting a line in the commit message🤔

@cruessler
Copy link
Contributor Author

@extrawurst I have incorporated your feedback, so the PR should be near feature-complete. The only thing missing are a few tests.

@extrawurst
Copy link
Owner

@cruessler really cool, a few tests would be good 👍

@cruessler
Copy link
Contributor Author

@extrawurst I just added a few tests. Let me know if you want more!

@cruessler cruessler marked this pull request as ready for review August 3, 2020 17:54
@extrawurst
Copy link
Owner

unfortunately it looks not right to me, something is off here:
Screenflick Movie 56

please checkout the amount of up arrows needed until it actually starts scrolling up and how it takes many down-arrow before it starts scrolling down again.

also the clip shows how the entire text is bold instead of only the first line (like it was before).

Can you look into that? You can reproduce it with 1c3295a in the gitui repo.

- Manually wrap commit message using `textwrap`

Closes extrawurst#181
@cruessler
Copy link
Contributor Author

@extrawurst Thanks for the feedback! My first solution was needlessly complicated because it tracked selection and scroll_top separately. I changed it so that it now only uses scroll_top which fixes the issues.

My terminal was not configured to show bold text, so I did not notice the other issue before. I fixed that, too.

@extrawurst extrawurst merged commit 1a90fd3 into extrawurst:master Aug 8, 2020
@extrawurst
Copy link
Owner

@cruessler really cool! thanks for having pulled through with this! great job!❤️

extrawurst pushed a commit that referenced this pull request Aug 9, 2020
@cruessler cruessler deleted the scroll-long-commit-message branch April 24, 2021 13:11
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.

2 participants