-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(ui): Use SelectControl in Context Picker Modal #8688
Conversation
// Focus this to get hotkeys to keep working | ||
let containerEl = document.querySelector('.main-container'); | ||
if (containerEl) { | ||
containerEl.focus(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems hacky, but not sure of a better solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it be done with ref like in
https://github.com/getsentry/sentry/blob/master/src/sentry/static/sentry/app/components/assigneeSelector.jsx#L292 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is when you close the modal, global hot keys stop working because the main body doesn't have focus. This is focusing an app container that's higher up in the tree. Ah after looking at it, .main-container
is the parent so we may be able to just use a callback, for some reason I thought it was higher up in the tree.
Use new SelectControl in Context Picker Modal.
8deab44
to
135dfe5
Compare
shouldHaveEmptyOrgSelector ? '' : latestContext.organization.slug | ||
} | ||
onChange={this.handleSelectOrganization} | ||
<React.Fragment> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: don't need this
if (!this.projectSelect || this.state.loading) return; | ||
|
||
// eslint-disable-next-line react/no-find-dom-node | ||
ReactDOM.findDOMNode(this.projectSelect) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to use forwardRef to clean this up, but there's some issues with react-test-renderer
serializing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* [ ] requires a PR in getsentry to be deployed together * [x] kill the old Account Notifications page before proceeding. Depends on: * ~~#8688 * ~~#8723 * ~~getsentry/getsentry#1961
* [ ] requires a PR in getsentry to be deployed together * [x] kill the old Account Notifications page before proceeding. Depends on: * ~~#8688 * ~~#8723 * ~~https://github.com/getsentry/getsentry/pull/1961~~
Use new SelectControl in Context Picker Modal.
Fix hot keys not working after closing modal.