Skip to content

Fix Enterprise URL Entry Box#100

Merged
pol-rivero merged 2 commits intopol-rivero:mainfrom
coocoo1112:cooperj/fixEnterpriseUrlEntryBox
Mar 24, 2026
Merged

Fix Enterprise URL Entry Box#100
pol-rivero merged 2 commits intopol-rivero:mainfrom
coocoo1112:cooperj/fixEnterpriseUrlEntryBox

Conversation

@coocoo1112
Copy link

@coocoo1112 coocoo1112 commented Mar 23, 2026

Summary

Fixes a bug where text typed into the "Enterprise or AE address" TextBox on the GitHub Enterprise sign-in page appears reversed (e.g. typing "https" displays "sptth").

Root Cause

This bug is caused by an interaction between the IME composition handling added in da0f1a10 ("Handle IME composition in text input components") and TextBox consumers that don't pass a value prop.

The IME changes added:

  • cursorPosition tracking in TextBox state
  • A componentDidUpdate that calls setSelectionRange() to restore cursor position, clamping to (this.state.value ?? '').length

The problem occurs in componentWillReceiveProps:

public componentWillReceiveProps(nextProps: ITextBoxProps) {
  if (this.state.value !== nextProps.value) {
    this.setState({ value: nextProps.value })
  }
}

When a parent component (like EnterpriseServerEntry) does not pass a value prop, nextProps.value is undefined. After each keystroke:

  1. onChange fires → sets state.value = 'h', cursorPosition = {start: 1, end: 1}
  2. onValueChanged callback triggers parent re-render
  3. Parent re-renders TextBox (still without a value prop)
  4. componentWillReceiveProps: this.state.value ('h') !== nextProps.value (undefined) → resets state.value to undefined
  5. componentDidUpdate: max = (undefined ?? '').length = 0safeStart = Math.min(1, 0) = 0setSelectionRange(0, 0) → cursor jumps to position 0
  6. Next keystroke inserts at position 0 → text builds up in reverse

This wasn't an issue before the IME changes because there was no componentDidUpdate reading state.value to position the cursor. The upstream desktop/desktop repo is also unaffected since it doesn't have the IME composition handling.

Fix

Add an undefined guard in componentWillReceiveProps:

public componentWillReceiveProps(nextProps: ITextBoxProps) {
  if (nextProps.value !== undefined && this.state.value !== nextProps.value) {
    this.setState({ value: nextProps.value })
  }
}

This prevents state.value from being reset to undefined when the parent simply doesn't pass a value prop, which means the cursor position clamping in componentDidUpdate uses the correct string length. This protects all uncontrolled TextBox consumers, not just the enterprise entry form.

Cooper Jones added 2 commits March 23, 2026 14:31
…Add missing value prop to the TextBox in EnterpriseServerEntry so the displayed text stays in sync with component state.
@coocoo1112
Copy link
Author

This was all Claude but it does fix the problem for me.

Currently it looks like this when you type https

image

@coocoo1112
Copy link
Author

My first commit also fixed it, fwiw, but it seemed better to touch code that was exclusive to this repo. Up to you though.

@pol-rivero
Copy link
Owner

Works like a charm, thanks!

it seemed better to touch code that was exclusive to this repo

Absolutely, it's always better to minimize the diff with the upstream.

@pol-rivero pol-rivero merged commit 70eb6ff into pol-rivero:main Mar 24, 2026
4 of 9 checks passed
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