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

cockpit-components-terminal: change onFocusIn/OnFocusOut to onFocus/onBlur #9342

Conversation

mareklibra
Copy link
Contributor

@mareklibra mareklibra commented Jun 8, 2018

Part of #9263 PR-split, see individual commits for more info.

@mareklibra mareklibra changed the title cockpit-components-terminal: Replace onFocusIn/onFocusOut by onFocus/… cockpit-components-terminal: React-related fixes Jun 8, 2018
@mareklibra mareklibra mentioned this pull request Jun 8, 2018
44 tasks
onFocusIn={this.onFocusIn}
onFocusOut={this.onFocusOut} />;
onFocus={this.onFocusIn}
onBlur={this.onFocusOut} />;
Copy link
Member

Choose a reason for hiding this comment

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

These events don't work with react-lite, the handler is never called. So either both properties should be set for a transitional period, or this is blocked on actually switching to React. At that point the handler methods should be renamed accordingly, too.

@martinpitt martinpitt added the blocked Don't land until something else happens first (see task list) label Jun 11, 2018
@mareklibra mareklibra changed the title cockpit-components-terminal: React-related fixes cockpit-components-terminal: change onFocusIn/OnFocusOut to onFocus/onBlur Jun 11, 2018
@mareklibra
Copy link
Contributor Author

Let's keep it blocked till #9263 lands.

@martinpitt martinpitt removed the blocked Don't land until something else happens first (see task list) label Sep 23, 2018
@martinpitt
Copy link
Member

#9263 landed now, unblocking.

@martinpitt martinpitt force-pushed the react.cockpit-components-terminal branch from 7e77b96 to 4bad7a5 Compare September 23, 2018 07:15
@martinpitt
Copy link
Member

Rebased to current master and adjusted commit log a bit. This might even be a regression from the react switch, thus priority.

@martinpitt martinpitt added the release-blocker Targetted for next release label Sep 23, 2018
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

I tested this manually and confirm that this brings back the proper interception of Ctrl+W.

@martinpitt martinpitt merged commit 6a0e49f into cockpit-project:master Sep 23, 2018
garrett pushed a commit to garrett/cockpit that referenced this pull request Oct 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Targetted for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants