Fix multiple file options showing up (#987) #988
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change ensures that the file options composite widget is hidden if and only if none of its components is in focus. This is achieved by having a variable keep track of the state of the composite widget (in focus / not in focus).
catarak
requested changes
Mar 26, 2019
Modified fix for processing#987 according to the requested changes. Moved isFocused to the components state and created a method for hadling the function calls executed in onBlur.
catarak
requested changes
Mar 27, 2019
| @@ -156,6 +168,8 @@ export class FileNode extends React.Component { | |||
| ref={(element) => { this[`fileOptions-${this.props.id}`] = element; }} | |||
| tabIndex="0" | |||
| onClick={this.toggleFileOptions} | |||
| onBlur={this.blurComponent} | |||
| onFocus={() => { this.setState({ isFocused: true }); }} | |||
There was a problem hiding this comment.
could you move this into a function called onfocusComponent?
There was a problem hiding this comment.
Definitely! I will fix this now.
| @@ -156,6 +168,8 @@ export class FileNode extends React.Component { | |||
| ref={(element) => { this[`fileOptions-${this.props.id}`] = element; }} | |||
| tabIndex="0" | |||
| onClick={this.toggleFileOptions} | |||
| onBlur={this.blurComponent} | |||
There was a problem hiding this comment.
could you call this function onBlurComponent rather than blurComponent?
There was a problem hiding this comment.
Yes! Thank you for the feedback.
|
thanks for making the changes! i've asked for a couple more organizational things but this is looking great. |
Renamed method blurComponent to onBlurComponent. Moved duplicated code from onFocus callback to a new method called onFocusComponent.
|
this is working great for me. merging! |
catarak
approved these changes
Mar 27, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change ensures that the file options composite widget is
hidden if and only if none of its components is in focus. This
is achieved by having a variable keep track of the state of
the composite widget (in focus / not in focus).
I have verified that this pull request:
npm run lint)Fixes #123