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

Fix left panel scrollbar when pasting long text in commit summary #17472

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

sergiou87
Copy link
Member

Closes #17320
Supersedes #17466

Description

While testing #17466 I noticed that didn't fix the issue for me, but the problem was clearly related to pasting long strings into the commit summary textbox. And then it was easily solved when a single character was entered or removed.

The problem seemed to be that the caret coordinates were updated immediately, but not the textbox inner scroll, so the textbox scroll X position remained at 0, but the caret, after pasting the long text, was far in the X axis, out of the boundaries of the textbox.

This PR includes two changes to address this issue:

  1. Make sure the caret coordinates (in the component state) is updated also when the user scrolls within the textbox contents.
  2. Add overflow: hidden to the textbox so even when the invisible caret is rightfully out of the boundaries of the textbox, Desktop doesn't show scrollbars.

Screenshots

Screen.Recording.2023-10-02.at.08.43.10.mov

Release notes

Notes: [Fixed] Pasting long texts in the commit summary textbox does not show a scrollbar in the left pane

Copy link
Contributor

@tidy-dev tidy-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ Works great! I love that it is a more targeted solution than what I suggested in the other PR.

One thing I was thinking tho is that, this PR targets the root cause, my suggestion targets the side panel behaving as expected regardless of the behavior of it's children. Do you think it would still make sense to add the overflow-x:hidden to the side panel just in case some other child component eventually misbehaves?

Approving this any which way as I think it is a plenty proper solution. :)

@sergiou87
Copy link
Member Author

I think that could hide other potential issues in specific components, so at least right now I'd rather keep the side panel unchanged 🤔

@sergiou87 sergiou87 merged commit 6313995 into development Oct 3, 2023
7 checks passed
@sergiou87 sergiou87 deleted the fix-commit-scrollbar branch October 3, 2023 13:26
@@ -4,6 +4,11 @@
position: relative;
display: flex;
flex-direction: column;

// Overflow must be hidden to prevent the "invisible caret" to generate an

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [ ~~~~

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.

An unexpected scrollbar was generated on the Commit Panel.
3 participants